From: Tim Deegan Date: Thu, 2 Jun 2011 12:16:52 +0000 (+0100) Subject: x86/mm/p2m: merge gfn_to_mfn_unshare with other gfn_to_mfn paths. X-Git-Tag: archive/raspbian/4.8.0-1+rpi1~1^2~10238 X-Git-Url: https://dgit.raspbian.org/?a=commitdiff_plain;h=8c7d0b26207932a07061b03e1873dc3c8987f568;p=xen.git x86/mm/p2m: merge gfn_to_mfn_unshare with other gfn_to_mfn paths. gfn_to_mfn_unshare() had its own function despite all other lookup types being handled in one place. Merge it into _gfn_to_mfn_type(), so that it gets the benefit of broken-page protection, for example, and tidy its interfaces up to fit. The unsharing code still has a lot of bugs, e.g. - failure to alloc for unshare on a foreign lookup still BUG()s, - at least one race condition in unshare-and-retry - p2m_* lookup types should probably be flags, not enum but it's cleaner and will make later p2m cleanups easier. Signed-off-by: Tim Deegan --- diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c index 1230de6031..ce51354324 100644 --- a/xen/arch/x86/hvm/emulate.c +++ b/xen/arch/x86/hvm/emulate.c @@ -63,7 +63,7 @@ static int hvmemul_do_io( int rc; /* Check for paged out page */ - ram_mfn = gfn_to_mfn_unshare(p2m, ram_gfn, &p2mt, 0); + ram_mfn = gfn_to_mfn_unshare(p2m, ram_gfn, &p2mt); if ( p2m_is_paging(p2mt) ) { p2m_mem_paging_populate(p2m, ram_gfn); diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index b53fd7f159..3e4568f2e1 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -352,7 +352,7 @@ static int hvm_set_ioreq_page( unsigned long mfn; void *va; - mfn = mfn_x(gfn_to_mfn_unshare(p2m, gmfn, &p2mt, 0)); + mfn = mfn_x(gfn_to_mfn_unshare(p2m, gmfn, &p2mt)); if ( !p2m_is_ram(p2mt) ) return -EINVAL; if ( p2m_is_paging(p2mt) ) @@ -1767,7 +1767,7 @@ static void *__hvm_map_guest_frame(unsigned long gfn, bool_t writable) struct p2m_domain *p2m = p2m_get_hostp2m(current->domain); mfn = mfn_x(writable - ? gfn_to_mfn_unshare(p2m, gfn, &p2mt, 0) + ? gfn_to_mfn_unshare(p2m, gfn, &p2mt) : gfn_to_mfn(p2m, gfn, &p2mt)); if ( (p2m_is_shared(p2mt) && writable) || !p2m_is_ram(p2mt) ) return NULL; @@ -2229,7 +2229,7 @@ static enum hvm_copy_result __hvm_copy( gfn = addr >> PAGE_SHIFT; } - mfn = mfn_x(gfn_to_mfn_unshare(p2m, gfn, &p2mt, 0)); + mfn = mfn_x(gfn_to_mfn_unshare(p2m, gfn, &p2mt)); if ( p2m_is_paging(p2mt) ) { @@ -3724,7 +3724,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg) rc = -EINVAL; if ( is_hvm_domain(d) ) { - gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t, 0); + gfn_to_mfn_unshare(p2m_get_hostp2m(d), a.pfn, &t); if ( p2m_is_mmio(t) ) a.mem_type = HVMMEM_mmio_dm; else if ( p2m_is_readonly(t) ) @@ -3779,7 +3779,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg) p2m_type_t t; p2m_type_t nt; mfn_t mfn; - mfn = gfn_to_mfn_unshare(p2m, pfn, &t, 0); + mfn = gfn_to_mfn_unshare(p2m, pfn, &t); if ( p2m_is_paging(t) ) { p2m_mem_paging_populate(p2m, pfn); @@ -3877,7 +3877,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE(void) arg) mfn_t mfn; int success; - mfn = gfn_to_mfn_unshare(p2m, pfn, &t, 0); + mfn = gfn_to_mfn_unshare(p2m, pfn, &t); p2m_lock(p2m); success = p2m->set_entry(p2m, pfn, mfn, 0, t, memaccess[a.hvmmem_access]); diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c index 609618f542..8c70a01d7c 100644 --- a/xen/arch/x86/mm.c +++ b/xen/arch/x86/mm.c @@ -4653,7 +4653,7 @@ long arch_memory_op(int op, XEN_GUEST_HANDLE(void) arg) p2m_type_t p2mt; xatp.idx = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d), - xatp.idx, &p2mt, 0)); + xatp.idx, &p2mt)); /* If the page is still shared, exit early */ if ( p2m_is_shared(p2mt) ) { diff --git a/xen/arch/x86/mm/guest_walk.c b/xen/arch/x86/mm/guest_walk.c index b20fdaff8b..4e4c295f8d 100644 --- a/xen/arch/x86/mm/guest_walk.c +++ b/xen/arch/x86/mm/guest_walk.c @@ -93,7 +93,7 @@ static inline void *map_domain_gfn(struct p2m_domain *p2m, uint32_t *rc) { /* Translate the gfn, unsharing if shared */ - *mfn = gfn_to_mfn_unshare(p2m, gfn_x(gfn), p2mt, 0); + *mfn = gfn_to_mfn_unshare(p2m, gfn_x(gfn), p2mt); if ( p2m_is_paging(*p2mt) ) { p2m_mem_paging_populate(p2m, gfn_x(gfn)); diff --git a/xen/arch/x86/mm/hap/guest_walk.c b/xen/arch/x86/mm/hap/guest_walk.c index cb8548222f..fda008d57e 100644 --- a/xen/arch/x86/mm/hap/guest_walk.c +++ b/xen/arch/x86/mm/hap/guest_walk.c @@ -57,7 +57,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)( walk_t gw; /* Get the top-level table's MFN */ - top_mfn = gfn_to_mfn_unshare(p2m, cr3 >> PAGE_SHIFT, &p2mt, 0); + top_mfn = gfn_to_mfn_unshare(p2m, cr3 >> PAGE_SHIFT, &p2mt); if ( p2m_is_paging(p2mt) ) { p2m_mem_paging_populate(p2m, cr3 >> PAGE_SHIFT); @@ -89,7 +89,7 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAGING_LEVELS)( if ( missing == 0 ) { gfn_t gfn = guest_l1e_get_gfn(gw.l1e); - gfn_to_mfn_unshare(p2m, gfn_x(gfn), &p2mt, 0); + gfn_to_mfn_unshare(p2m, gfn_x(gfn), &p2mt); if ( p2m_is_paging(p2mt) ) { p2m_mem_paging_populate(p2m, gfn_x(gfn)); diff --git a/xen/arch/x86/mm/mem_sharing.c b/xen/arch/x86/mm/mem_sharing.c index bf1940dd65..f3f5f25b42 100644 --- a/xen/arch/x86/mm/mem_sharing.c +++ b/xen/arch/x86/mm/mem_sharing.c @@ -294,8 +294,7 @@ static void mem_sharing_audit(void) static struct page_info* mem_sharing_alloc_page(struct domain *d, - unsigned long gfn, - int must_succeed) + unsigned long gfn) { struct page_info* page; struct vcpu *v = current; @@ -307,22 +306,21 @@ static struct page_info* mem_sharing_alloc_page(struct domain *d, memset(&req, 0, sizeof(req)); req.type = MEM_EVENT_TYPE_SHARED; - if(must_succeed) + if ( v->domain != d ) { - /* We do not support 'must_succeed' any more. External operations such - * as grant table mappings may fail with OOM condition! - */ - BUG(); - } - else - { - /* All foreign attempts to unshare pages should be handled through - * 'must_succeed' case. */ - ASSERT(v->domain->domain_id == d->domain_id); - vcpu_pause_nosync(v); - req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED; + /* XXX This path needs some attention. For now, just fail foreign + * XXX requests to unshare if there's no memory. This replaces + * XXX old code that BUG()ed here; the callers now BUG() + * XXX elewhere. */ + gdprintk(XENLOG_ERR, + "Failed alloc on unshare path for foreign (%d) lookup\n", + d->domain_id); + return page; } + vcpu_pause_nosync(v); + req.flags |= MEM_EVENT_FLAG_VCPU_PAUSED; + /* XXX: Need to reserve a request, not just check the ring! */ if(mem_event_check_ring(d)) return page; @@ -692,8 +690,7 @@ gfn_found: if(ret == 0) goto private_page_found; old_page = page; - page = mem_sharing_alloc_page(d, gfn, flags & MEM_SHARING_MUST_SUCCEED); - BUG_ON(!page && (flags & MEM_SHARING_MUST_SUCCEED)); + page = mem_sharing_alloc_page(d, gfn); if(!page) { /* We've failed to obtain memory for private page. Need to re-add the diff --git a/xen/common/grant_table.c b/xen/common/grant_table.c index e3ad8e60c2..ba32a358a5 100644 --- a/xen/common/grant_table.c +++ b/xen/common/grant_table.c @@ -110,7 +110,8 @@ static unsigned inline int max_nr_maptrack_frames(void) #define gfn_to_mfn_private(_d, _gfn) ({ \ p2m_type_t __p2mt; \ unsigned long __x; \ - __x = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(_d), _gfn, &__p2mt, 1)); \ + __x = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(_d), _gfn, &__p2mt)); \ + BUG_ON(p2m_is_shared(__p2mt)); /* XXX fixme */ \ if ( !p2m_is_valid(__p2mt) ) \ __x = INVALID_MFN; \ __x; }) @@ -153,7 +154,12 @@ static int __get_paged_frame(unsigned long gfn, unsigned long *frame, int readon if ( readonly ) mfn = gfn_to_mfn(p2m, gfn, &p2mt); else - mfn = gfn_to_mfn_unshare(p2m, gfn, &p2mt, 1); + { + mfn = gfn_to_mfn_unshare(p2m, gfn, &p2mt); + BUG_ON(p2m_is_shared(p2mt)); + /* XXX Here, and above in gfn_to_mfn_private, need to handle + * XXX failure to unshare. */ + } if ( p2m_is_valid(p2mt) ) { *frame = mfn_x(mfn); diff --git a/xen/common/memory.c b/xen/common/memory.c index cf6482de9f..382caf6c51 100644 --- a/xen/common/memory.c +++ b/xen/common/memory.c @@ -363,7 +363,7 @@ static long memory_exchange(XEN_GUEST_HANDLE(xen_memory_exchange_t) arg) p2m_type_t p2mt; /* Shared pages cannot be exchanged */ - mfn = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d), gmfn + k, &p2mt, 0)); + mfn = mfn_x(gfn_to_mfn_unshare(p2m_get_hostp2m(d), gmfn + k, &p2mt)); if ( p2m_is_shared(p2mt) ) { rc = -ENOMEM; diff --git a/xen/include/asm-x86/mem_sharing.h b/xen/include/asm-x86/mem_sharing.h index 3dcfec6a74..f465bb7abd 100644 --- a/xen/include/asm-x86/mem_sharing.h +++ b/xen/include/asm-x86/mem_sharing.h @@ -34,7 +34,6 @@ int mem_sharing_nominate_page(struct p2m_domain *p2m, unsigned long gfn, int expected_refcnt, shr_handle_t *phandle); -#define MEM_SHARING_MUST_SUCCEED (1<<0) #define MEM_SHARING_DESTROY_GFN (1<<1) int mem_sharing_unshare_page(struct p2m_domain *p2m, unsigned long gfn, diff --git a/xen/include/asm-x86/p2m.h b/xen/include/asm-x86/p2m.h index a3b4e43b00..66e32a9037 100644 --- a/xen/include/asm-x86/p2m.h +++ b/xen/include/asm-x86/p2m.h @@ -112,9 +112,10 @@ typedef enum { } p2m_access_t; typedef enum { - p2m_query = 0, /* Do not populate a PoD entries */ - p2m_alloc = 1, /* Automatically populate PoD entries */ - p2m_guest = 2, /* Guest demand-fault; implies alloc */ + p2m_query, /* Do not populate a PoD entries */ + p2m_alloc, /* Automatically populate PoD entries */ + p2m_unshare, /* Break c-o-w sharing; implies alloc */ + p2m_guest, /* Guest demand-fault; implies alloc */ } p2m_query_t; /* We use bitmaps and maks to handle groups of types */ @@ -366,6 +367,14 @@ gfn_to_mfn_type_p2m(struct p2m_domain *p2m, unsigned long gfn, { mfn_t mfn = p2m->get_entry(p2m, gfn, t, a, q); +#ifdef __x86_64__ + if ( q == p2m_unshare && p2m_is_shared(*t) ) + { + mem_sharing_unshare_page(p2m, gfn, 0); + mfn = p2m->get_entry(p2m, gfn, t, a, q); + } +#endif + #ifdef __x86_64__ if (unlikely((p2m_is_broken(*t)))) { @@ -401,32 +410,7 @@ static inline mfn_t _gfn_to_mfn_type(struct p2m_domain *p2m, #define gfn_to_mfn(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), p2m_alloc) #define gfn_to_mfn_query(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), p2m_query) #define gfn_to_mfn_guest(p2m, g, t) _gfn_to_mfn_type((p2m), (g), (t), p2m_guest) - -static inline mfn_t gfn_to_mfn_unshare(struct p2m_domain *p2m, - unsigned long gfn, - p2m_type_t *p2mt, - int must_succeed) -{ - mfn_t mfn; - - mfn = gfn_to_mfn(p2m, gfn, p2mt); -#ifdef __x86_64__ - if ( p2m_is_shared(*p2mt) ) - { - if ( mem_sharing_unshare_page(p2m, gfn, - must_succeed - ? MEM_SHARING_MUST_SUCCEED : 0) ) - { - BUG_ON(must_succeed); - return mfn; - } - mfn = gfn_to_mfn(p2m, gfn, p2mt); - } -#endif - - return mfn; -} - +#define gfn_to_mfn_unshare(p, g, t) _gfn_to_mfn_type((p), (g), (t), p2m_unshare) /* Compatibility function exporting the old untyped interface */ static inline unsigned long gmfn_to_mfn(struct domain *d, unsigned long gpfn)