p2m synchronization groundwork
PATCH 1/1 p2m synchronization groundwork
This patch series lays the groundwork for improving the synchronization of primitives accessing the p2m.
This is a partial repost of the patches emailed previously as a RFC. These patches are now intended for committing to the tree.
We change the API for accessing the p2m to a family of functions get_gfn/put_gfn. The name intends to reflect the fact that even lookups are meant to obtain exclusive access to a p2m entry, and that said access should be relinquished (put_gfn) when done.
The patches, however, alter little functionality. The API name change does not involve yet additional locking or ref-counting. They will, however, throw a "barrier" that will force any new commits to conform to the new API.
Patches are based off 24066:54a5e994a241. Should the new XENMEM calls be accepted before this, the series needs to be updated to also change the API there.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/mm/mm-locks.h | 27 ++++- xen/arch/x86/mm/mm-locks.h | 46 +++++++++++ xen/arch/x86/mm/mm-locks.h | 12 ++ xen/arch/x86/mm/p2m-pod.c | 40 ++++++--- xen/include/asm-x86/domain.h | 3 + xen/include/asm-x86/p2m.h | 5 + xen/arch/x86/mm/p2m.c | 38 +++++++++ xen/include/asm-x86/p2m.h | 40 +--------- xen/arch/x86/cpu/mcheck/vmce.c | 9 +- xen/arch/x86/debug.c | 17 ++- xen/arch/x86/domain.c | 27 +++++- xen/arch/x86/domctl.c | 15 ++- xen/arch/x86/hvm/emulate.c | 29 ++++++- xen/arch/x86/hvm/hvm.c | 133 +++++++++++++++++++++++++------ xen/arch/x86/hvm/mtrr.c | 2 +- xen/arch/x86/hvm/nestedhvm.c | 2 +- xen/arch/x86/hvm/stdvga.c | 4 +- xen/arch/x86/hvm/svm/nestedsvm.c | 12 +- xen/arch/x86/hvm/svm/svm.c | 11 +- xen/arch/x86/hvm/viridian.c | 8 +- xen/arch/x86/hvm/vmx/vmx.c | 15 ++- xen/arch/x86/hvm/vmx/vvmx.c | 13 ++- xen/arch/x86/mm.c | 153 +++++++++++++++++++++++++++++------- xen/arch/x86/mm/guest_walk.c | 30 +++++- xen/arch/x86/mm/hap/guest_walk.c | 16 ++- xen/arch/x86/mm/hap/nested_hap.c | 15 ++- xen/arch/x86/mm/mem_event.c | 23 ++++- xen/arch/x86/mm/mem_sharing.c | 27 +++++- xen/arch/x86/mm/p2m-pod.c | 19 ++- xen/arch/x86/mm/p2m-pt.c | 6 +- xen/arch/x86/mm/p2m.c | 34 ++++--- xen/arch/x86/mm/shadow/common.c | 6 +- xen/arch/x86/mm/shadow/multi.c | 85 ++++++++++++++----- xen/arch/x86/mm/shadow/types.h | 10 +- xen/arch/x86/physdev.c | 8 +- xen/arch/x86/traps.c | 19 +++- xen/common/grant_table.c | 30 +++++- xen/common/memory.c | 13 ++- xen/common/tmem_xen.c | 21 +++- xen/include/asm-ia64/mm.h | 2 + xen/include/asm-x86/guest_pt.h | 6 +- xen/include/asm-x86/hvm/hvm.h | 5 +- xen/include/asm-x86/hvm/vmx/vvmx.h | 1 + xen/include/asm-x86/p2m.h | 38 ++++++-- 44 files changed, 799 insertions(+), 276 deletions(-)
PATCH 1/1 Refactor mm-lock ordering constructs
xen/arch/x86/mm/mm-locks.h | 27 +++++++++++++++++++-------- 1 files changed, 19 insertions(+), 8 deletions(-)
The mm layer has a construct to enforce locks are taken in a pre- defined order, and thus avert deadlock. Refactor pieces of this code for later use, no functional changes.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/mm/mm-locks.h +19 -8
@@ -28,6 +28,7 @@/* Per-CPU variable for enforcing the lock ordering */ DECLARE_PER_CPU (int, mm_lock_level); +#define __get_lock_level () (this_cpu(mm_lock_level))static inline void mm_lock_init (mm_lock_t *l) {
@@ -42,22 +43,32 @@ static inline int mm_locked_by_me(mm_locreturn (l->lock.recurse_cpu == current->processor); } +/* If you see this crash, the numbers printed are lines in this file +* where the offending locks are declared. */ +#define __check_lock_level (l) \ +do { \ + if ( unlikely(__get_lock_level()) > (l) ) \ + panic("mm locking order violation: %i > %i\n", \ + __get_lock_level(), (l)); \ +} while(0) + +#define __set_lock_level (l) \ +do { \ + __get_lock_level() = (l); \ +} while(0) +static inline void _mm_lock (mm_lock_t *l,const char *func,int level,int rec) { -/* If you see this crash, the numbers printed are lines in this file -* where the offending locks are declared. */ -if (unlikely (this_cpu (mm_lock_level) > level) ) -panic ("mm locking order violation: %i > %i\n" , -this_cpu (mm_lock_level), level); +__check_lock_level (level);spin_lock_recursive (&l->lock);if ( l->lock.recurse_cnt ==1 ) { l->locker_function = func; - l->unlock_level =this_cpu (mm_lock_level); + l->unlock_level =__get_lock_level (); }else if ( (unlikely (!rec)) )panic ("mm lock already held by %s\n" , l->locker_function); -this_cpu (mm_lock_level) = level; +__set_lock_level (level); }/* This wrapper uses the line number to express the locking order below */ #define declare_mm_lock (name) \
@@ -72,7 +83,7 @@ static inline void mm_unlock(mm_lock_t *if ( l->lock.recurse_cnt ==1 ) { l->locker_function ="nobody" ; -this_cpu (mm_lock_level) = l->unlock_level; +__set_lock_level (l->unlock_level); }spin_unlock_recursive (&l->lock); }
PATCH 1/1 Declare an order-enforcing construct for external locks used in the mm layer
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mm-locks.h | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 46 insertions(+), 0 deletions(-)
Declare an order-enforcing construct for a lock used in the mm layer that is not of type mm_lock_t. This is useful whenever the mm layer takes locks from other subsystems, or locks not implemented as mm_lock_t.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/mm/mm-locks.h +46 -0
@@ -70,6 +70,27 @@ static inline void _mm_lock(mm_lock_t *lpanic ("mm lock already held by %s\n" , l->locker_function);__set_lock_level (level); } + +static inline void _mm_enforce_order_lock_pre (int level) +{ +__check_lock_level (level); +} + +static inline void _mm_enforce_order_lock_post (int level,int *unlock_level, +unsigned short *recurse_count) +{ +if ( recurse_count ) + { +if ( *recurse_count++ ==0 ) + { + *unlock_level =__get_lock_level (); + } + }else { + *unlock_level =__get_lock_level (); + } +__set_lock_level (level); +} +/* This wrapper uses the line number to express the locking order below */ #define declare_mm_lock (name) \ static inline void mm_lock_##name(mm_lock_t *l, const char *func, int rec)\
@@ -78,6 +99,16 @@ static inline void _mm_lock(mm_lock_t *l#define mm_lock (name, l) mm_lock_##name(l, __func__, 0)#define mm_lock_recursive (name, l) mm_lock_##name(l, __func__, 1) +/* This wrapper is intended for "external" locks which do not use +* the mm_lock_t types. Such locks inside the mm code are also subject +* to ordering constraints. */ +#define declare_mm_order_constraint (name) \ + static inline void mm_enforce_order_lock_pre_##name(void) \ + { _mm_enforce_order_lock_pre(__LINE__); } \ + static inline void mm_enforce_order_lock_post_##name( \ + int *unlock_level, unsigned short *recurse_count) \ + { _mm_enforce_order_lock_post(__LINE__, unlock_level, recurse_count); } \ +static inline void mm_unlock (mm_lock_t *l) {if ( l->lock.recurse_cnt ==1 )
@@ -88,6 +119,21 @@ static inline void mm_unlock(mm_lock_t *spin_unlock_recursive (&l->lock); } +static inline void mm_enforce_order_unlock (int unlock_level, +unsigned short *recurse_count) +{ +if ( recurse_count ) + { +BUG_ON (*recurse_count ==0 ); +if ( *recurse_count-- ==1 ) + { +__set_lock_level (unlock_level); + } + }else { +__set_lock_level (unlock_level); + } +} +/************************************************************************ * * * To avoid deadlocks, these locks _MUST_ be taken in the order they're *
PATCH 1/1 Enforce ordering constraints for the page alloc lock in the PoD code
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mm-locks.h | 12 ++++++++++++ xen/arch/x86/mm/p2m-pod.c | 40 +++++++++++++++++++++++++++------------- xen/include/asm-x86/domain.h | 3 +++ xen/include/asm-x86/p2m.h | 5 +++++ 4 files changed, 47 insertions(+), 13 deletions(-)
The page alloc lock is sometimes used in the PoD code, with an explicit expectation of ordering. Use our ordering constructs in the mm layer to enforce this.
The additional book-keeping variables are kept in the arch_domain sub-struct, as they are x86-specific to the whole domain.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/mm/mm-locks.h +12 -0
@@ -174,6 +174,18 @@ declare_mm_lock(p2m)#define p2m_unlock (p) mm_unlock(&(p)->lock)#define p2m_locked_by_me (p) mm_locked_by_me(&(p)->lock) +/* Page alloc lock (per-domain) +* +* This is an external lock, not represented by an mm_lock_t. However, +* pod code uses it in conjunction with the p2m lock, and expecting +* the ordering which we enforce here. +* The lock is not recursive. */ + +declare_mm_order_constraint(page_alloc) +#define page_alloc_mm_pre_lock () mm_enforce_order_lock_pre_page_alloc() +#define page_alloc_mm_post_lock (l) mm_enforce_order_lock_post_page_alloc(&(l), NULL) +#define page_alloc_mm_unlock (l) mm_enforce_order_unlock((l), NULL) +/* Paging lock (per-domain) * * For shadow pagetables, this lock protects
xen/arch/x86/mm/p2m-pod.c +27 -13
@@ -45,6 +45,20 @@#define superpage_aligned (_x) (((_x)&(SUPERPAGE_PAGES-1))==0) +/* Enforce lock ordering when grabbing the "external" page_alloc lock */ +static inline void lock_page_alloc (struct p2m_domain *p2m) +{ +page_alloc_mm_pre_lock (); +spin_lock (&(p2m->domain->page_alloc_lock)); +page_alloc_mm_post_lock (p2m->domain->arch.page_alloc_unlock_level); +} + +static inline void unlock_page_alloc (struct p2m_domain *p2m) +{ +page_alloc_mm_unlock (p2m->domain->arch.page_alloc_unlock_level); +spin_unlock (&(p2m->domain->page_alloc_lock)); +} +/* * Populate-on-demand functionality */
@@ -100,7 +114,7 @@ p2m_pod_cache_add(struct p2m_domain *p2munmap_domain_page (b); } -spin_lock (&d->page_alloc_lock); +lock_page_alloc (p2m);/* First, take all pages off the domain list */ for (i=0 ; i <1 << order ; i++)
@@ -128,7 +142,7 @@ p2m_pod_cache_add(struct p2m_domain *p2m* This may cause "zombie domains" since the page will never be freed. */ BUG_ON ( d->arch.relmem != RELMEM_not_started ); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);return 0 ; }
@@ -245,7 +259,7 @@ p2m_pod_set_cache_target(struct p2m_doma/* Grab the lock before checking that pod.super is empty, or the last * entries may disappear before we grab the lock. */ -spin_lock (&d->page_alloc_lock); +lock_page_alloc (p2m);if ( (p2m->pod.count - pod_target) >SUPERPAGE_PAGES && !page_list_empty (&p2m->pod.super) )
@@ -257,7 +271,7 @@ p2m_pod_set_cache_target(struct p2m_domaASSERT (page !=NULL ); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);/* Then free them */ for ( i =0 ; i < (1 << order) ; i++ )
@@ -378,7 +392,7 @@ p2m_pod_empty_cache(struct domain *d)BUG_ON (!d->is_dying);spin_barrier (&p2m->lock.lock); -spin_lock (&d->page_alloc_lock); +lock_page_alloc (p2m);while ( (page =page_list_remove_head (&p2m->pod.super)) ) {
@@ -403,7 +417,7 @@ p2m_pod_empty_cache(struct domain *d)BUG_ON (p2m->pod.count !=0 ); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m); }int
@@ -417,7 +431,7 @@ p2m_pod_offline_or_broken_hit(struct pagif ( !(d =page_get_owner (p)) || !(p2m =p2m_get_hostp2m (d)) )return 0 ; -spin_lock (&d->page_alloc_lock); +lock_page_alloc (p2m); bmfn =mfn_x (page_to_mfn (p));page_list_for_each_safe (q, tmp, &p2m->pod.super) {
@@ -448,12 +462,12 @@ p2m_pod_offline_or_broken_hit(struct pag } } -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);return 0 ; pod_hit:page_list_add_tail (p, &d->arch.relmem_list); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);return 1 ; }
@@ -994,7 +1008,7 @@ p2m_pod_demand_populate(struct p2m_domaiif ( q == p2m_guest && gfn > p2m->pod.max_guest ) p2m->pod.max_guest = gfn; -spin_lock (&d->page_alloc_lock); +lock_page_alloc (p2m);if ( p2m->pod.count ==0 ) goto out_of_memory;
@@ -1008,7 +1022,7 @@ p2m_pod_demand_populate(struct p2m_domaiBUG_ON ((mfn_x (mfn) & ((1 << order)-1 )) !=0 ); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m); gfn_aligned = (gfn >> order) << order;
@@ -1040,7 +1054,7 @@ p2m_pod_demand_populate(struct p2m_domaireturn 0 ; out_of_memory: -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);printk ("%s: Out of populate-on-demand memory! tot_pages %" PRIu32" pod_entries %" PRIi32"\n" , __func__, d->tot_pages, p2m->pod.entry_count);
@@ -1049,7 +1063,7 @@ out_fail:return -1 ; remap_and_retry:BUG_ON (order !=PAGE_ORDER_2M ); -spin_unlock (&d->page_alloc_lock); +unlock_page_alloc (p2m);/* Remap this 2-meg region in singleton chunks */ gfn_aligned = (gfn>>order)<<order;
xen/include/asm-x86/domain.h +3 -0
@@ -269,6 +269,9 @@ struct arch_domainstruct paging_domain paging;struct p2m_domain *p2m; +/* To enforce lock ordering in the pod code wrt the +* page_alloc lock */ +int page_alloc_unlock_level;/* nestedhvm: translate l2 guest physical to host physical */ struct p2m_domain *nested_p2m[MAX_NESTEDP2M ];
xen/include/asm-x86/p2m.h +5 -0
@@ -270,6 +270,11 @@ struct p2m_domain {* + p2m_pod_demand_populate() grabs both; the p2m lock to avoid * double-demand-populating of pages, the page_alloc lock to * protect moving stuff from the PoD cache to the domain page list. +* +* We enforce this lock ordering through a construct in mm-locks.h. +* This demands, however, that we store the previous lock-ordering +* level in effect before grabbing the page_alloc lock. The unlock +* level is stored in the arch section of the domain struct. */ struct { struct page_list_head super, /* List of superpages */
PATCH 1/1 Refactor p2m get_entry accessor
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/p2m.c | 38 ++++++++++++++++++++++++++++++++++++++ xen/include/asm-x86/p2m.h | 40 ++-------------------------------------- 2 files changed, 40 insertions(+), 38 deletions(-)
Move the main query accessor to the p2m outside of an inline and into the p2m code itself. This will allow for p2m internal locking to be added to the accessor later.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/mm/p2m.c +38 -0
@@ -144,6 +144,44 @@ void p2m_change_entry_type_global(structp2m_unlock (p2m); } +mfn_t gfn_to_mfn_type_p2m (struct p2m_domain *p2m,unsigned long gfn, +p2m_type_t *t,p2m_access_t *a,p2m_query_t q, +unsigned int *page_order) +{ +mfn_t mfn; + +if ( !p2m || !paging_mode_translate (p2m->domain) ) + { +/* Not necessarily true, but for non-translated guests, we claim +* it's the most generic kind of memory */ + *t = p2m_ram_rw; +return _mfn (gfn); + } + + mfn = p2m->get_entry (p2m, gfn, t, a, q, page_order); + +#ifdef __x86_64__ +if ( q == p2m_unshare &&p2m_is_shared (*t) ) + { +ASSERT (!p2m_is_nestedp2m (p2m)); +mem_sharing_unshare_page (p2m->domain, gfn,0 ); + mfn = p2m->get_entry (p2m, gfn, t, a, q, page_order); + } +#endif + +#ifdef __x86_64__ +if (unlikely ((p2m_is_broken (*t)))) + { +/* Return invalid_mfn to avoid caller's access */ + mfn =_mfn (INVALID_MFN ); +if (q == p2m_guest) +domain_crash (p2m->domain); + } +#endif + +return mfn; +} +int set_p2m_entry (struct p2m_domain *p2m,unsigned long gfn,mfn_t mfn,unsigned int page_order,p2m_type_t p2mt,p2m_access_t p2ma) {
xen/include/asm-x86/p2m.h +2 -38
@@ -313,45 +313,9 @@ struct p2m_domain *p2m_get_p2m(struct vc* If the lookup succeeds, the return value is != INVALID_MFN and * *page_order is filled in with the order of the superpage (if any) that * the entry was found in. */ -static inline mfn_t -gfn_to_mfn_type_p2m (struct p2m_domain *p2m,unsigned long gfn, +mfn_t gfn_to_mfn_type_p2m (struct p2m_domain *p2m,unsigned long gfn,p2m_type_t *t,p2m_access_t *a,p2m_query_t q, -unsigned int *page_order) -{ -mfn_t mfn; - -if ( !p2m || !paging_mode_translate (p2m->domain) ) - { -/* Not necessarily true, but for non-translated guests, we claim -* it's the most generic kind of memory */ - *t = p2m_ram_rw; -return _mfn (gfn); - } - - mfn = p2m->get_entry (p2m, gfn, t, a, q, page_order); - -#ifdef __x86_64__ -if ( q == p2m_unshare &&p2m_is_shared (*t) ) - { -ASSERT (!p2m_is_nestedp2m (p2m)); -mem_sharing_unshare_page (p2m->domain, gfn,0 ); - mfn = p2m->get_entry (p2m, gfn, t, a, q, page_order); - } -#endif - -#ifdef __x86_64__ -if (unlikely ((p2m_is_broken (*t)))) - { -/* Return invalid_mfn to avoid caller's access */ - mfn =_mfn (INVALID_MFN ); -if (q == p2m_guest) -domain_crash (p2m->domain); - } -#endif - -return mfn; -} - +unsigned int *page_order);/* General conversion function from gfn to mfn */ static inline mfn_t gfn_to_mfn_type(struct domain *d,
PATCH 1/1 Modify naming of queries into the p2m
In reply to Andres Lagar-Cavillaxen/arch/x86/cpu/mcheck/vmce.c | 9 +- xen/arch/x86/debug.c | 17 ++- xen/arch/x86/domain.c | 27 +++++- xen/arch/x86/domctl.c | 15 ++- xen/arch/x86/hvm/emulate.c | 29 ++++++- xen/arch/x86/hvm/hvm.c | 133 +++++++++++++++++++++++++------ xen/arch/x86/hvm/mtrr.c | 2 +- xen/arch/x86/hvm/nestedhvm.c | 2 +- xen/arch/x86/hvm/stdvga.c | 4 +- xen/arch/x86/hvm/svm/nestedsvm.c | 12 +- xen/arch/x86/hvm/svm/svm.c | 11 +- xen/arch/x86/hvm/viridian.c | 8 +- xen/arch/x86/hvm/vmx/vmx.c | 15 ++- xen/arch/x86/hvm/vmx/vvmx.c | 13 ++- xen/arch/x86/mm.c | 153 +++++++++++++++++++++++++++++------- xen/arch/x86/mm/guest_walk.c | 30 +++++- xen/arch/x86/mm/hap/guest_walk.c | 16 ++- xen/arch/x86/mm/hap/nested_hap.c | 15 ++- xen/arch/x86/mm/mem_event.c | 23 ++++- xen/arch/x86/mm/mem_sharing.c | 27 +++++- xen/arch/x86/mm/p2m-pod.c | 19 ++- xen/arch/x86/mm/p2m-pt.c | 6 +- xen/arch/x86/mm/p2m.c | 34 ++++--- xen/arch/x86/mm/shadow/common.c | 6 +- xen/arch/x86/mm/shadow/multi.c | 85 ++++++++++++++----- xen/arch/x86/mm/shadow/types.h | 10 +- xen/arch/x86/physdev.c | 8 +- xen/arch/x86/traps.c | 19 +++- xen/common/grant_table.c | 30 +++++- xen/common/memory.c | 13 ++- xen/common/tmem_xen.c | 21 +++- xen/include/asm-ia64/mm.h | 2 + xen/include/asm-x86/guest_pt.h | 6 +- xen/include/asm-x86/hvm/hvm.h | 5 +- xen/include/asm-x86/hvm/vmx/vvmx.h | 1 + xen/include/asm-x86/p2m.h | 38 ++++++-- 36 files changed, 647 insertions(+), 217 deletions(-)
Callers of lookups into the p2m code are now variants of get_gfn. All callers need to call put_gfn. The code behind it is a no-op at the moment, but will change to proper locking in a later patch.
This patch does not change functionality. Only naming, and adds put_gfn's.
set_p2m_entry retains its name because it is always called with p2m_lock held.
This patch is humongous, unfortunately, given the dozens of call sites involved.
After this patch, anyone using old style gfn_to_mfn will not succeed in compiling their code. This is on purpose: adapt to the new API.
Signed-off-by Andres Lagar-Cavilla <[email protected]>
xen/arch/x86/cpu/mcheck/vmce.c +6 -3
@@ -574,6 +574,7 @@ int unmmap_broken_page(struct domain *d, {mfn_t r_mfn;p2m_type_t pt; +int rc;/* Always trust dom0's MCE handler will prevent future access */ if ( d == dom0 )
@@ -585,14 +586,16 @@ int unmmap_broken_page(struct domain *d,if ( !is_hvm_domain (d) || !paging_mode_hap (d) )return -ENOSYS ; - r_mfn =gfn_to_mfn_query (d, gfn, &pt); + rc =-1 ; + r_mfn =get_gfn_query (d, gfn, &pt);if (p2m_to_mask (pt) &P2M_UNMAP_TYPES ) {ASSERT (mfn_x (r_mfn) ==mfn_x (mfn));p2m_change_type (d, gfn, pt, p2m_ram_broken); -return 0 ; + rc =0 ; } +put_gfn (d, gfn); -return -1 ; +return rc; }
xen/arch/x86/debug.c +10 -7
@@ -43,22 +43,23 @@/* Returns: mfn for the given (hvm guest) vaddr */ static unsigned long -dbg_hvm_va2mfn (dbgva_t vaddr,struct domain *dp,int toaddr) +dbg_hvm_va2mfn (dbgva_t vaddr,struct domain *dp,int toaddr, +unsigned long *gfn) { -unsigned long mfn, gfn; +unsigned long mfn;uint32_t pfec = PFEC_page_present;p2m_type_t gfntype;DBGP2 ("vaddr:%lx domid:%d\n" , vaddr, dp->domain_id); - gfn =paging_gva_to_gfn (dp->vcpu[0 ], vaddr, &pfec); -if ( gfn ==INVALID_GFN ) + *gfn =paging_gva_to_gfn (dp->vcpu[0 ], vaddr, &pfec); +if ( *gfn ==INVALID_GFN ) {DBGP2 ("kdb:bad gfn from gva_to_gfn\n" );return INVALID_MFN ; } - mfn =mfn_x (gfn_to_mfn (dp, gfn, &gfntype)); + mfn =mfn_x (get_gfn (dp, *gfn, &gfntype));if (p2m_is_readonly (gfntype) && toaddr ) {DBGP2 ("kdb:p2m_is_readonly: gfntype:%x\n" , gfntype);
@@ -193,12 +194,12 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_twhile ( len >0 ) {char *va; -unsigned long mfn, pagecnt; +unsigned long mfn, gfn =INVALID_GFN , pagecnt; pagecnt =min_t (long,PAGE_SIZE - (addr & ~PAGE_MASK ), len); mfn = (dp->is_hvm - ?dbg_hvm_va2mfn (addr, dp, toaddr) + ?dbg_hvm_va2mfn (addr, dp, toaddr, &gfn) :dbg_pv_va2mfn (addr, dp, pgd3));if ( mfn ==INVALID_MFN )break ;
@@ -217,6 +218,8 @@ dbg_rw_guest_mem(dbgva_t addr, dbgbyte_t }unmap_domain_page (va); +if ( gfn !=INVALID_GFN ) +put_gfn (dp, gfn); addr += pagecnt; buf += pagecnt;
xen/arch/x86/domain.c +21 -6
@@ -720,6 +720,7 @@ int arch_set_info_guest( struct vcpu *v, vcpu_guest_context_u c) {struct domain *d = v->domain; +unsigned long cr3_gfn;unsigned long cr3_pfn =INVALID_MFN ;unsigned long flags, cr4;unsigned int i;
@@ -931,7 +932,8 @@ int arch_set_info_guest(if ( !compat ) { - cr3_pfn =gmfn_to_mfn (d,xen_cr3_to_pfn (c.nat->ctrlreg[3 ])); + cr3_gfn =xen_cr3_to_pfn (c.nat->ctrlreg[3 ]); + cr3_pfn =get_gfn_untyped (d, cr3_gfn); if ( !mfn_valid(cr3_pfn) || (paging_mode_refcounts(d)
@@ -939,16 +941,18 @@ int arch_set_info_guest( : !get_page_and_type(mfn_to_page(cr3_pfn), d, PGT_base_page_table)) ) { +put_gfn (d, cr3_gfn);destroy_gdt (v);return -EINVAL ; } v->arch.guest_table =pagetable_from_pfn (cr3_pfn); - +put_gfn (d, cr3_gfn);#ifdef __x86_64__if ( c.nat->ctrlreg[1 ] ) { - cr3_pfn =gmfn_to_mfn (d,xen_cr3_to_pfn (c.nat->ctrlreg[1 ])); + cr3_gfn =xen_cr3_to_pfn (c.nat->ctrlreg[1 ]); + cr3_pfn =get_gfn_untyped (d, cr3_gfn); if ( !mfn_valid(cr3_pfn) || (paging_mode_refcounts(d)
@@ -962,11 +966,13 @@ int arch_set_info_guest(put_page (mfn_to_page (cr3_pfn)); elseput_page_and_type (mfn_to_page (cr3_pfn )); +put_gfn (d, cr3_gfn);destroy_gdt (v);return -EINVAL ; } v->arch.guest_table_user =pagetable_from_pfn (cr3_pfn); +put_gfn (d, cr3_gfn); }else if ( !(flags & VGCF_in_kernel) ) {
@@ -978,7 +984,8 @@ int arch_set_info_guest( {l4_pgentry_t *l4tab; - cr3_pfn =gmfn_to_mfn (d,compat_cr3_to_pfn (c.cmp->ctrlreg[3 ])); + cr3_gfn =compat_cr3_to_pfn (c.cmp->ctrlreg[3 ]); + cr3_pfn =get_gfn_untyped (d, cr3_gfn); if ( !mfn_valid(cr3_pfn) || (paging_mode_refcounts(d)
@@ -986,6 +993,7 @@ int arch_set_info_guest( : !get_page_and_type(mfn_to_page(cr3_pfn), d, PGT_l3_page_table)) ) { +put_gfn (d, cr3_gfn);destroy_gdt (v);return -EINVAL ; }
@@ -993,6 +1001,7 @@ int arch_set_info_guest( l4tab =__va (pagetable_get_paddr (v->arch.guest_table)); *l4tab =l4e_from_pfn ( cr3_pfn, _PAGE_PRESENT|_PAGE_RW|_PAGE_USER|_PAGE_ACCESSED); +put_gfn (d, cr3_gfn);#endif }
@@ -1058,11 +1067,12 @@ unmap_vcpu_info(struct vcpu *v)* event doesn't get missed. */ static int -map_vcpu_info (struct vcpu *v,unsigned long mfn,unsigned offset) +map_vcpu_info (struct vcpu *v,unsigned long gfn,unsigned offset) {struct domain *d = v->domain;void *mapping;vcpu_info_t *new_info; +unsigned long mfn;int i;if ( offset > (PAGE_SIZE -sizeof (vcpu_info_t)) )
@@ -1075,15 +1085,19 @@ map_vcpu_info(struct vcpu *v, unsigned lif ( (v != current) && !test_bit (_VPF_down, &v->pause_flags) )return -EINVAL ; - mfn =gmfn_to_mfn (d, mfn); + mfn =get_gfn_untyped (d, gfn);if ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), d, PGT_writable_page) ) + { +put_gfn (d, gfn);return -EINVAL ; + } mapping =map_domain_page_global (mfn);if ( mapping ==NULL ) {put_page_and_type (mfn_to_page (mfn)); +put_gfn (d, gfn);return -ENOMEM ; }
@@ -1113,6 +1127,7 @@ map_vcpu_info(struct vcpu *v, unsigned lfor ( i =0 ; i <BITS_PER_EVTCHN_WORD (d); i++ )set_bit (i, &vcpu_info (v, evtchn_pending_sel)); +put_gfn (d, gfn);return 0 ; }
xen/arch/x86/domctl.c +11 -4
@@ -199,7 +199,7 @@ long arch_do_domctl(for ( j =0 ; j < k; j++ ) { -unsigned long type =0 , mfn =gmfn_to_mfn (d, arr[j]); +unsigned long type =0 , mfn =get_gfn_untyped (d, arr[j]); page =mfn_to_page (mfn);
@@ -235,6 +235,7 @@ long arch_do_domctl( type =XEN_DOMCTL_PFINFO_XTAB ; arr[j] = type; +put_gfn (d, arr[j]); } if ( copy_to_guest_offset(domctl->u.getpageframeinfo3.array,
@@ -299,7 +300,8 @@ long arch_do_domctl(for ( j =0 ; j < k; j++ ) {struct page_info *page; -unsigned long mfn =gmfn_to_mfn (d, arr32[j]); +unsigned long gfn = arr32[j]; +unsigned long mfn =get_gfn_untyped (d, gfn); page =mfn_to_page (mfn);
@@ -310,8 +312,10 @@ long arch_do_domctl( unlikely(is_xen_heap_mfn(mfn)) ) arr32[j] |=XEN_DOMCTL_PFINFO_XTAB ; else if ( xsm_getpageframeinfo(page) != 0 ) + { +put_gfn (d, gfn);continue ; - else if ( likely(get_page(page, d)) ) + } else if ( likely(get_page(page, d)) ) {unsigned long type =0 ;
@@ -339,6 +343,7 @@ long arch_do_domctl(else arr32[j] |=XEN_DOMCTL_PFINFO_XTAB ; +put_gfn (d, gfn); } if ( copy_to_guest_offset(domctl->u.getpageframeinfo2.array,
@@ -425,12 +430,13 @@ long arch_do_domctl(break ; } - mfn =gmfn_to_mfn (d, gmfn); + mfn =get_gfn_untyped (d, gmfn); ret = -EACCES ;if ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), d, PGT_writable_page) ) { +put_gfn (d, gmfn);rcu_unlock_domain (d);break ; }
@@ -443,6 +449,7 @@ long arch_do_domctl(put_page_and_type (mfn_to_page (mfn)); +put_gfn (d, gmfn);rcu_unlock_domain (d); }break ;
xen/arch/x86/hvm/emulate.c +26 -3
@@ -63,14 +63,18 @@ static int hvmemul_do_io(int rc;/* Check for paged out page */ - ram_mfn =gfn_to_mfn_unshare (curr->domain, ram_gfn, &p2mt); + ram_mfn =get_gfn_unshare (curr->domain, ram_gfn, &p2mt);if (p2m_is_paging (p2mt) ) {p2m_mem_paging_populate (curr->domain, ram_gfn); +put_gfn (curr->domain, ram_gfn);return X86EMUL_RETRY ; }if (p2m_is_shared (p2mt) ) + { +put_gfn (curr->domain, ram_gfn);return X86EMUL_RETRY ; + }/* * Weird-sized accesses have undefined behaviour: we discard writes
@@ -82,6 +86,7 @@ static int hvmemul_do_io(ASSERT (p_data !=NULL );/* cannot happen with a REP prefix */ if ( dir ==IOREQ_READ )memset (p_data, ~0 , size); +put_gfn (curr->domain, ram_gfn);return X86EMUL_UNHANDLEABLE ; }
@@ -101,7 +106,10 @@ static int hvmemul_do_io(paddr_t pa = vio->mmio_large_write_pa;unsigned int bytes = vio->mmio_large_write_bytes;if ( (addr >= pa) && ((addr + size) <= (pa + bytes)) ) + { +put_gfn (curr->domain, ram_gfn);return X86EMUL_OKAY ; + } }else {
@@ -111,6 +119,7 @@ static int hvmemul_do_io( {memcpy (p_data, &vio->mmio_large_read[addr - pa], size); +put_gfn (curr->domain, ram_gfn);return X86EMUL_OKAY ; } }
@@ -123,15 +132,22 @@ static int hvmemul_do_io(case HVMIO_completed: vio->io_state = HVMIO_none;if ( p_data ==NULL ) + { +put_gfn (curr->domain, ram_gfn);return X86EMUL_UNHANDLEABLE ; + } goto finish_access;case HVMIO_dispatched:/* May have to wait for previous cycle of a multi-write to complete. */ if ( is_mmio && !value_is_ptr && (dir ==IOREQ_WRITE ) && (addr == (vio->mmio_large_write_pa + vio->mmio_large_write_bytes)) ) + { +put_gfn (curr->domain, ram_gfn);return X86EMUL_RETRY ; + }default : +put_gfn (curr->domain, ram_gfn);return X86EMUL_UNHANDLEABLE ; }
@@ -139,6 +155,7 @@ static int hvmemul_do_io( {gdprintk (XENLOG_WARNING ,"WARNING: io already pending (%d)?\n" , p->state); +put_gfn (curr->domain, ram_gfn);return X86EMUL_UNHANDLEABLE ; }
@@ -189,7 +206,10 @@ static int hvmemul_do_io( }if ( rc !=X86EMUL_OKAY ) + { +put_gfn (curr->domain, ram_gfn);return rc; + } finish_access:if ( p_data !=NULL )
@@ -223,6 +243,7 @@ static int hvmemul_do_io( } } +put_gfn (curr->domain, ram_gfn);return X86EMUL_OKAY ; }
@@ -671,12 +692,14 @@ static int hvmemul_rep_movs(if ( rc !=X86EMUL_OKAY )return rc; - (void )gfn_to_mfn (current->domain, sgpa >>PAGE_SHIFT , &p2mt); +/* Unlocked works here because we get_gfn for real in whatever +* we call later. */ + (void )get_gfn_unlocked (current->domain, sgpa >>PAGE_SHIFT , &p2mt);if ( !p2m_is_ram (p2mt) && !p2m_is_grant (p2mt) )return hvmemul_do_mmio ( sgpa, reps, bytes_per_rep, dgpa,IOREQ_READ , df,NULL ); - (void )gfn_to_mfn (current->domain, dgpa >>PAGE_SHIFT , &p2mt); + (void )get_gfn_unlocked (current->domain, dgpa >>PAGE_SHIFT , &p2mt);if ( !p2m_is_ram (p2mt) && !p2m_is_grant (p2mt) )return hvmemul_do_mmio ( dgpa, reps, bytes_per_rep, sgpa,IOREQ_WRITE , df,NULL );
xen/arch/x86/hvm/hvm.c +105 -28
389 lines not on this page · raw message
xen/arch/x86/hvm/mtrr.c +1 -1
@@ -389,7 +389,7 @@ uint32_t get_pat_flags(struct vcpu *v, {struct domain *d = v->domain;p2m_type_t p2mt; -gfn_to_mfn_query (d,paddr_to_pfn (gpaddr), &p2mt); +get_gfn_query_unlocked (d,paddr_to_pfn (gpaddr), &p2mt);if (p2m_is_ram (p2mt))gdprintk (XENLOG_WARNING , "Conflict occurs for a given guest l1e flags:%x "
xen/arch/x86/hvm/nestedhvm.c +1 -1
@@ -56,7 +56,7 @@ nestedhvm_vcpu_reset(struct vcpu *v) nv->nv_ioportED =0 ;if (nv->nv_vvmcx) -hvm_unmap_guest_frame (nv->nv_vvmcx); +hvm_unmap_guest_frame (nv->nv_vvmcx, nv->nv_vvmcxaddr >>PAGE_SHIFT ,0 ); nv->nv_vvmcx =NULL ; nv->nv_vvmcxaddr =VMCX_EADDR ; nv->nv_flushp2m =0 ;
xen/arch/x86/hvm/stdvga.c +2 -2
@@ -482,7 +482,7 @@ static int mmio_move(struct hvm_hw_stdvgif (hvm_copy_to_guest_phys (data, &tmp, p->size) != HVMCOPY_okay ) { - (void )gfn_to_mfn (d, data >>PAGE_SHIFT , &p2mt); + (void )get_gfn_unlocked (d, data >>PAGE_SHIFT , &p2mt);/* * The only case we handle is vga_mem <-> vga_mem. * Anything else disables caching and leaves it to qemu-dm.
@@ -504,7 +504,7 @@ static int mmio_move(struct hvm_hw_stdvgif (hvm_copy_from_guest_phys (&tmp, data, p->size) != HVMCOPY_okay ) { - (void )gfn_to_mfn (d, data >>PAGE_SHIFT , &p2mt); + (void )get_gfn_unlocked (d, data >>PAGE_SHIFT , &p2mt);if ( (p2mt != p2m_mmio_dm) || (data <VGA_MEM_BASE ) || ((data + p->size) > (VGA_MEM_BASE +VGA_MEM_SIZE )) )return 0 ;
xen/arch/x86/hvm/svm/nestedsvm.c +7 -5
@@ -71,7 +71,7 @@ int nestedsvm_vmcb_map(struct vcpu *v, uif (nv->nv_vvmcx !=NULL && nv->nv_vvmcxaddr != vmcbaddr) {ASSERT (nv->nv_vvmcx !=NULL );ASSERT (nv->nv_vvmcxaddr !=VMCX_EADDR ); -hvm_unmap_guest_frame (nv->nv_vvmcx); +hvm_unmap_guest_frame (nv->nv_vvmcx, nv->nv_vvmcxaddr >>PAGE_SHIFT ,0 ); nv->nv_vvmcx =NULL ; nv->nv_vvmcxaddr =VMCX_EADDR ; }
@@ -353,7 +353,7 @@ static int nsvm_vmrun_permissionmap(struASSERT (ns_viomap !=NULL ); ioport_80 =test_bit (0x80 , ns_viomap); ioport_ed =test_bit (0xed , ns_viomap); -hvm_unmap_guest_frame (ns_viomap); +hvm_unmap_guest_frame (ns_viomap, svm->ns_iomap_pa >>PAGE_SHIFT ,0 ); svm->ns_iomap =nestedhvm_vcpu_iomap_get (ioport_80, ioport_ed);
@@ -857,23 +857,25 @@ nsvm_vmcb_guest_intercepts_ioio(paddr_tioio_info_t ioinfo;uint16_t port;bool_t enabled; +unsigned long gfn =0 ;/* gcc ... */ ioinfo.bytes = exitinfo1; port = ioinfo.fields.port;switch (port) { case 0 ... 32767: /* first 4KB page */ - io_bitmap =hvm_map_guest_frame_ro (iopm_gfn); + gfn = iopm_gfn;break ; case 32768 ... 65535: /* second 4KB page */ port -=32768 ; - io_bitmap =hvm_map_guest_frame_ro (iopm_gfn+1 ); + gfn = iopm_gfn +1 ;break ;default :BUG ();break ; } + io_bitmap =hvm_map_guest_frame_ro (gfn);if (io_bitmap ==NULL ) {gdprintk (XENLOG_ERR ,"IOIO intercept: mapping of permission map failed\n" );
@@ -881,7 +883,7 @@ nsvm_vmcb_guest_intercepts_ioio(paddr_t } enabled =test_bit (port, io_bitmap); -hvm_unmap_guest_frame (io_bitmap); +hvm_unmap_guest_frame (io_bitmap, gfn,0 );if (!enabled)return NESTEDHVM_VMEXIT_HOST ;
xen/arch/x86/hvm/svm/svm.c +7 -4
@@ -244,9 +244,10 @@ static int svm_vmcb_restore(struct vcpu {if ( c->cr0 &X86_CR0_PG ) { - mfn =mfn_x (gfn_to_mfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) || !get_page (mfn_to_page (mfn), v->domain) ) { +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT );gdprintk (XENLOG_ERR ,"Invalid CR3 value=0x%" PRIx64"\n" , c->cr3);return -EINVAL ;
@@ -257,6 +258,8 @@ static int svm_vmcb_restore(struct vcpuput_page (pagetable_get_page (v->arch.guest_table)); v->arch.guest_table =pagetable_from_pfn (mfn); +if ( c->cr0 &X86_CR0_PG ) +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT ); } v->arch.hvm_vcpu.guest_cr[0 ] = c->cr0 |X86_CR0_ET ;
@@ -1144,7 +1147,6 @@ static void svm_do_nested_pgfault(structunsigned long gfn = gpa >>PAGE_SHIFT ;mfn_t mfn;p2m_type_t p2mt; -p2m_access_t p2ma;struct p2m_domain *p2m =NULL ; ret =hvm_hap_nested_page_fault (gpa,0 , ~0ul ,0 ,0 ,0 ,0 );
@@ -1161,7 +1163,8 @@ static void svm_do_nested_pgfault(struct p2m =p2m_get_p2m (v); _d.gpa = gpa; _d.qualification =0 ; - _d.mfn =mfn_x (gfn_to_mfn_type_p2m (p2m, gfn, &_d.p2mt, &p2ma, p2m_query,NULL )); + mfn =get_gfn_query_unlocked (p2m->domain, gfn, &_d.p2mt); + _d.mfn =mfn_x (mfn);__trace_var (TRC_HVM_NPF ,0 ,sizeof (_d), &_d); }
@@ -1181,7 +1184,7 @@ static void svm_do_nested_pgfault(structif ( p2m ==NULL ) p2m =p2m_get_p2m (v);/* Everything else is an error. */ - mfn =gfn_to_mfn_type_p2m (p2m, gfn, &p2mt, &p2ma, p2m_guest,NULL ); + mfn =get_gfn_guest_unlocked (p2m->domain, gfn, &p2mt);gdprintk (XENLOG_ERR ,"SVM violation gpa %#" PRIpaddr", mfn %#lx, type %i\n" , gpa,mfn_x (mfn), p2mt);
xen/arch/x86/hvm/viridian.c +6 -2
@@ -134,12 +134,13 @@ void dump_apic_assist(struct vcpu *v)static void enable_hypercall_page (struct domain *d) {unsigned long gmfn = d->arch.hvm_domain.viridian.hypercall_gpa.fields.pfn; -unsigned long mfn =gmfn_to_mfn (d, gmfn); +unsigned long mfn =get_gfn_untyped (d, gmfn);uint8_t *p;if ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), d, PGT_writable_page) ) { +put_gfn (d, gmfn);gdprintk (XENLOG_WARNING ,"Bad GMFN %lx (MFN %lx)\n" , gmfn, mfn);return ; }
@@ -162,13 +163,14 @@ static void enable_hypercall_page(structunmap_domain_page (p);put_page_and_type (mfn_to_page (mfn)); +put_gfn (d, gmfn); }void initialize_apic_assist (struct vcpu *v) {struct domain *d = v->domain;unsigned long gmfn = v->arch.hvm_vcpu.viridian.apic_assist.fields.pfn; -unsigned long mfn =gmfn_to_mfn (d, gmfn); +unsigned long mfn =get_gfn_untyped (d, gmfn);uint8_t *p;/*
@@ -184,6 +186,7 @@ void initialize_apic_assist(struct vcpuif ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), d, PGT_writable_page) ) { +put_gfn (d, gmfn);gdprintk (XENLOG_WARNING ,"Bad GMFN %lx (MFN %lx)\n" , gmfn, mfn);return ; }
@@ -195,6 +198,7 @@ void initialize_apic_assist(struct vcpuunmap_domain_page (p);put_page_and_type (mfn_to_page (mfn)); +put_gfn (d, gmfn); }int wrmsr_viridian_regs (uint32_t idx,uint64_t val)
xen/arch/x86/hvm/vmx/vmx.c +11 -4
@@ -487,9 +487,10 @@ static int vmx_restore_cr0_cr3( {if ( cr0 &X86_CR0_PG ) { - mfn =mfn_x (gfn_to_mfn (v->domain, cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) || !get_page (mfn_to_page (mfn), v->domain) ) { +put_gfn (v->domain, cr3 >>PAGE_SHIFT );gdprintk (XENLOG_ERR ,"Invalid CR3 value=0x%lx\n" , cr3);return -EINVAL ; }
@@ -499,6 +500,8 @@ static int vmx_restore_cr0_cr3(put_page (pagetable_get_page (v->arch.guest_table)); v->arch.guest_table =pagetable_from_pfn (mfn); +if ( cr0 &X86_CR0_PG ) +put_gfn (v->domain, cr3 >>PAGE_SHIFT ); } v->arch.hvm_vcpu.guest_cr[0 ] = cr0 |X86_CR0_ET ;
@@ -1007,9 +1010,12 @@ static void vmx_load_pdptrs(struct vcpuif ( cr3 &0x1fUL ) goto crash; - mfn =mfn_x (gfn_to_mfn (v->domain, cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) ) + { +put_gfn (v->domain, cr3 >>PAGE_SHIFT ); goto crash; + } p =map_domain_page (mfn);
@@ -1037,6 +1043,7 @@ static void vmx_load_pdptrs(struct vcpuvmx_vmcs_exit (v);unmap_domain_page (p); +put_gfn (v->domain, cr3 >>PAGE_SHIFT );return ; crash:
@@ -2088,7 +2095,7 @@ static void ept_handle_violation(unsigne _d.gpa = gpa; _d.qualification = qualification; - _d.mfn =mfn_x (gfn_to_mfn_query (d, gfn, &_d.p2mt)); + _d.mfn =mfn_x (get_gfn_query_unlocked (d, gfn, &_d.p2mt));__trace_var (TRC_HVM_NPF ,0 ,sizeof (_d), &_d); }
@@ -2104,7 +2111,7 @@ static void ept_handle_violation(unsignereturn ;/* Everything else is an error. */ - mfn =gfn_to_mfn_guest (d, gfn, &p2mt); + mfn =get_gfn_guest_unlocked (d, gfn, &p2mt); gdprintk(XENLOG_ERR, "EPT violation %#lx (%c%c%c/%c%c%c), " "gpa %#"PRIpaddr", mfn %#lx, type %i.\n", qualification,
xen/arch/x86/hvm/vmx/vvmx.c +9 -4
@@ -558,8 +558,10 @@ static void __map_io_bitmap(struct vcpu index = vmcs_reg ==IO_BITMAP_A ?0 :1 ;if (nvmx->iobitmap[index]) -hvm_unmap_guest_frame (nvmx->iobitmap[index]); +hvm_unmap_guest_frame (nvmx->iobitmap[index], + nvmx->iobitmap_gfn[index],0 ); gpa =__get_vvmcs (vcpu_nestedhvm (v).nv_vvmcx, vmcs_reg); + nvmx->iobitmap_gfn[index] = gpa >>PAGE_SHIFT ; nvmx->iobitmap[index] =hvm_map_guest_frame_ro (gpa >>PAGE_SHIFT ); }
@@ -577,13 +579,16 @@ static void nvmx_purge_vvmcs(struct vcpu__clear_current_vvmcs (v);if ( nvcpu->nv_vvmcxaddr !=VMCX_EADDR ) -hvm_unmap_guest_frame (nvcpu->nv_vvmcx); +hvm_unmap_guest_frame (nvcpu->nv_vvmcx, + nvcpu->nv_vvmcxaddr >>PAGE_SHIFT ,0 ); nvcpu->nv_vvmcx ==NULL ; nvcpu->nv_vvmcxaddr =VMCX_EADDR ;for (i=0 ; i<2 ; i++) {if ( nvmx->iobitmap[i] ) { -hvm_unmap_guest_frame (nvmx->iobitmap[i]); +hvm_unmap_guest_frame (nvmx->iobitmap[i], + nvmx->iobitmap_gfn[i],0 ); nvmx->iobitmap[i] =NULL ; + nvmx->iobitmap_gfn[i] =0 ; } } }
@@ -1198,7 +1203,7 @@ int nvmx_handle_vmclear(struct cpu_user_ vvmcs =hvm_map_guest_frame_rw (gpa >>PAGE_SHIFT );if ( vvmcs )__set_vvmcs (vvmcs,NVMX_LAUNCH_STATE ,0 ); -hvm_unmap_guest_frame (vvmcs); +hvm_unmap_guest_frame (vvmcs, gpa >>PAGE_SHIFT ,0 ); }vmreturn (regs,VMSUCCEED );
xen/arch/x86/mm.c +124 -29
583 lines not on this page · raw message
xen/arch/x86/mm/guest_walk.c +24 -6
@@ -86,30 +86,33 @@ static uint32_t set_ad_bits(void *guest_return 0 ; } +/* If the map is non-NULL, we leave this function having +* called get_gfn, you need to call put_gfn. */ static inline void *map_domain_gfn (struct p2m_domain *p2m,gfn_t gfn,mfn_t *mfn,p2m_type_t *p2mt,uint32_t *rc) { -p2m_access_t a; -/* Translate the gfn, unsharing if shared */ - *mfn =gfn_to_mfn_type_p2m (p2m,gfn_x (gfn), p2mt, &a, p2m_unshare,NULL ); + *mfn =get_gfn_unshare (p2m->domain,gfn_x (gfn), p2mt);if (p2m_is_paging (*p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain,gfn_x (gfn)); +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_PAGED;return NULL ; }if (p2m_is_shared (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_SHARED;return NULL ; }if ( !p2m_is_ram (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc |= _PAGE_PRESENT;return NULL ; }
@@ -120,6 +123,9 @@ static inline void *map_domain_gfn(struc/* Walk the guest pagetables, after the manner of a hardware walker. */ +/* Because the walk is essentially random, it can cause a deadlock +* warning in the p2m locking code. Highly unlikely this is an actual +* deadlock, because who would walk page table in the opposite order? */ uint32_t guest_walk_tables(struct vcpu *v, struct p2m_domain *p2m, unsigned long va, walk_t *gw,
@@ -347,12 +353,24 @@ set_ad: out:#if GUEST_PAGING_LEVELS ==4 -if ( l3p )unmap_domain_page (l3p); +if ( l3p ) + { +unmap_domain_page (l3p); +put_gfn (p2m->domain,gfn_x (guest_l4e_get_gfn (gw->l4e))); + }#endif #if GUEST_PAGING_LEVELS >=3 -if ( l2p )unmap_domain_page (l2p); +if ( l2p ) + { +unmap_domain_page (l2p); +put_gfn (p2m->domain,gfn_x (guest_l3e_get_gfn (gw->l3e))); + }#endif -if ( l1p )unmap_domain_page (l1p); +if ( l1p ) + { +unmap_domain_page (l1p); +put_gfn (p2m->domain,gfn_x (guest_l2e_get_gfn (gw->l2e))); + }return rc; }
xen/arch/x86/mm/hap/guest_walk.c +12 -4
@@ -54,28 +54,31 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PAmfn_t top_mfn;void *top_map;p2m_type_t p2mt; -p2m_access_t p2ma;walk_t gw; +unsigned long top_gfn;/* Get the top-level table's MFN */ - top_mfn =gfn_to_mfn_type_p2m (p2m, cr3 >>PAGE_SHIFT , - &p2mt, &p2ma, p2m_unshare,NULL ); + top_gfn = cr3 >>PAGE_SHIFT ; + top_mfn =get_gfn_unshare (p2m->domain, top_gfn, &p2mt);if (p2m_is_paging (p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain, cr3 >>PAGE_SHIFT ); pfec[0 ] = PFEC_page_paged; +put_gfn (p2m->domain, top_gfn);return INVALID_GFN ; }if (p2m_is_shared (p2mt) ) { pfec[0 ] = PFEC_page_shared; +put_gfn (p2m->domain, top_gfn);return INVALID_GFN ; }if ( !p2m_is_ram (p2mt) ) { pfec[0 ] &= ~PFEC_page_present; +put_gfn (p2m->domain, top_gfn);return INVALID_GFN ; }
@@ -87,26 +90,31 @@ unsigned long hap_p2m_ga_to_gfn(GUEST_PA#endif missing =guest_walk_tables (v, p2m, ga, &gw, pfec[0 ], top_mfn, top_map);unmap_domain_page (top_map); +put_gfn (p2m->domain, top_gfn);/* Interpret the answer */ if ( missing ==0 ) {gfn_t gfn =guest_l1e_get_gfn (gw.l1e); -gfn_to_mfn_type_p2m (p2m,gfn_x (gfn), &p2mt, &p2ma, p2m_unshare,NULL ); + (void )get_gfn_unshare (p2m->domain,gfn_x (gfn), &p2mt);if (p2m_is_paging (p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain,gfn_x (gfn)); pfec[0 ] = PFEC_page_paged; +put_gfn (p2m->domain,gfn_x (gfn));return INVALID_GFN ; }if (p2m_is_shared (p2mt) ) { pfec[0 ] = PFEC_page_shared; +put_gfn (p2m->domain,gfn_x (gfn));return INVALID_GFN ; } +put_gfn (p2m->domain,gfn_x (gfn)); +if ( page_order ) *page_order =guest_walk_to_page_order (&gw);
xen/arch/x86/mm/hap/nested_hap.c +11 -4
@@ -146,22 +146,29 @@ nestedhap_walk_L0_p2m(struct p2m_domainmfn_t mfn;p2m_type_t p2mt;p2m_access_t p2ma; +int rc;/* walk L0 P2M table */ mfn =gfn_to_mfn_type_p2m (p2m, L1_gpa >>PAGE_SHIFT , &p2mt, &p2ma, p2m_query, page_order); + rc =NESTEDHVM_PAGEFAULT_MMIO ;if (p2m_is_mmio (p2mt) ) -return NESTEDHVM_PAGEFAULT_MMIO ; + goto out; + rc =NESTEDHVM_PAGEFAULT_ERROR ;if (p2m_is_paging (p2mt) ||p2m_is_shared (p2mt) || !p2m_is_ram (p2mt) ) -return NESTEDHVM_PAGEFAULT_ERROR ; + goto out; + rc =NESTEDHVM_PAGEFAULT_ERROR ;if ( !mfn_valid (mfn) ) -return NESTEDHVM_PAGEFAULT_ERROR ; + goto out; *L0_gpa = (mfn_x (mfn) <<PAGE_SHIFT ) + (L1_gpa & ~PAGE_MASK ); -return NESTEDHVM_PAGEFAULT_DONE ; + rc =NESTEDHVM_PAGEFAULT_DONE ; +out: +put_gfn (p2m->domain, L1_gpa >>PAGE_SHIFT ); +return rc; }/* This function uses L2_gpa to walk the P2M page table in L1. If the
xen/arch/x86/mm/mem_event.c +18 -5
@@ -47,7 +47,7 @@ static int mem_event_enable(struct domaiunsigned long ring_addr = mec->ring_addr;unsigned long shared_addr = mec->shared_addr;l1_pgentry_t l1e; -unsigned long gfn; +unsigned long shared_gfn =0 , ring_gfn =0 ;/* gcc ... */ p2m_type_t p2mt;mfn_t ring_mfn;mfn_t shared_mfn;
@@ -60,23 +60,36 @@ static int mem_event_enable(struct domai/* Get MFN of ring page */ guest_get_eff_l1e (v, ring_addr, &l1e); - gfn =l1e_get_pfn (l1e); - ring_mfn =gfn_to_mfn (dom_mem_event, gfn, &p2mt); + 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))) ) + { +put_gfn (dom_mem_event, ring_gfn);return -EINVAL ; + }/* Get MFN of shared page */ guest_get_eff_l1e (v, shared_addr, &l1e); - gfn =l1e_get_pfn (l1e); - shared_mfn =gfn_to_mfn (dom_mem_event, gfn, &p2mt); + 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 */ 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);/* Allocate event channel */ rc = alloc_unbound_xen_event_channel(d->vcpu[0],
xen/arch/x86/mm/mem_sharing.c +21 -6
@@ -227,7 +227,7 @@ static void mem_sharing_audit(void) g->domain, g->gfn, mfn_x(e->mfn));continue ; } - mfn =gfn_to_mfn (d, g->gfn, &t); + mfn =get_gfn_unlocked (d, g->gfn, &t);if (mfn_x (mfn) !=mfn_x (e->mfn)) MEM_SHARING_DEBUG("Incorrect P2M for d=%d, PFN=%lx." "Expecting MFN=%ld, got %ld\n",
@@ -335,7 +335,7 @@ int mem_sharing_debug_gfn(struct domainp2m_type_t p2mt;mfn_t mfn; - mfn =gfn_to_mfn (d, gfn, &p2mt); + mfn =get_gfn_unlocked (d, gfn, &p2mt); printk("Debug for domain=%d, gfn=%lx, ", d->domain_id,
@@ -460,7 +460,7 @@ int mem_sharing_nominate_page(struct dom *phandle =0UL ;shr_lock (); - mfn =gfn_to_mfn (d, gfn, &p2mt); + mfn =get_gfn (d, gfn, &p2mt);/* Check if mfn is valid */ ret = -EINVAL ;
@@ -524,6 +524,7 @@ int mem_sharing_nominate_page(struct dom ret =0 ; out: +put_gfn (d, gfn);shr_unlock ();return ret; }
@@ -593,14 +594,18 @@ int mem_sharing_unshare_page(struct domashr_handle_t handle;struct list_head *le; +/* Remove the gfn_info from the list */ + +/* This is one of the reasons why we can't enforce ordering +* between shr_lock and p2m fine-grained locks in mm-lock. +* Callers may walk in here already holding the lock for this gfn */ shr_lock ();mem_sharing_audit (); - -/* Remove the gfn_info from the list */ - mfn =gfn_to_mfn (d, gfn, &p2mt); + mfn =get_gfn (d, gfn, &p2mt);/* Has someone already unshared it? */ if (!p2m_is_shared (p2mt)) { +put_gfn (d, gfn);shr_unlock ();return 0 ; }
@@ -634,6 +639,7 @@ gfn_found:/* Even though we don't allocate a private page, we have to account * for the MFN that originally backed this PFN. */ atomic_dec (&nr_saved_mfns); +put_gfn (d, gfn);shr_unlock ();put_page_and_type (page); if(last_gfn &&
@@ -653,6 +659,7 @@ gfn_found:/* We've failed to obtain memory for private page. Need to re-add the * gfn_info to relevant list */ list_add (&gfn_info->list, &hash_entry->gfns); +put_gfn (d, gfn);shr_unlock ();return -ENOMEM ; }
@@ -663,6 +670,13 @@ gfn_found:unmap_domain_page (s);unmap_domain_page (t); +/* NOTE: set_shared_p2m_entry will switch the underlying mfn. If +* we do get_page withing get_gfn, the correct sequence here +* should be +get_page(page); +put_page(old_page); +* so that the ref to the old page is dropped, and a ref to +* the new page is obtained to later be dropped in put_gfn */ BUG_ON (set_shared_p2m_entry (d, gfn,page_to_mfn (page)) ==0 );put_page_and_type (old_page);
@@ -683,6 +697,7 @@ private_page_found:/* Update m2p entry */ set_gpfn_from_mfn (mfn_x (page_to_mfn (page)), gfn); +put_gfn (d, gfn);shr_unlock ();return 0 ; }
xen/arch/x86/mm/p2m-pod.c +12 -7
@@ -531,9 +531,10 @@ p2m_pod_decrease_reservation(struct doma/* FIXME: Add contiguous; query for PSE entries? */ for ( i=0 ; i<(1 <<order); i++) { +p2m_access_t a;p2m_type_t t; -gfn_to_mfn_query (d, gpfn + i, &t); + (void )p2m->get_entry (p2m, gpfn + i, &t, &a, p2m_query,NULL );if ( t == p2m_populate_on_demand ) pod++;
@@ -572,8 +573,9 @@ p2m_pod_decrease_reservation(struct doma {mfn_t mfn;p2m_type_t t; +p2m_access_t a; - mfn =gfn_to_mfn_query (d, gpfn + i, &t); + mfn = p2m->get_entry (p2m, gpfn + i, &t, &a, p2m_query,NULL );if ( t == p2m_populate_on_demand ) {set_p2m_entry (p2m, gpfn + i,_mfn (INVALID_MFN ),0 , p2m_invalid, p2m->default_access);
@@ -653,8 +655,8 @@ p2m_pod_zero_check_superpage(struct p2m_* and aligned, and mapping them. */ for ( i=0 ; i<SUPERPAGE_PAGES ; i++ ) { - - mfn =gfn_to_mfn_query (d, gfn + i, &type); +p2m_access_t a; + mfn = p2m->get_entry (p2m, gfn + i, &type, &a, p2m_query,NULL );if ( i ==0 ) {
@@ -782,7 +784,8 @@ p2m_pod_zero_check(struct p2m_domain *p2/* First, get the gfn list, translate to mfns, and map the pages. */ for ( i=0 ; i<count; i++ ) { - mfns[i] =gfn_to_mfn_query (d, gfns[i], types + i); +p2m_access_t a; + mfns[i] = p2m->get_entry (p2m, gfns[i], types + i, &a, p2m_query,NULL );/* If this is ram, and not a pagetable or from the xen heap, and probably not mapped elsewhere, map it; otherwise, skip. */ if ( p2m_is_ram(types[i])
@@ -923,7 +926,8 @@ p2m_pod_emergency_sweep(struct p2m_domai/* FIXME: Figure out how to avoid superpages */ for ( i=p2m->pod.reclaim_single; i >0 ; i-- ) { -gfn_to_mfn_query (p2m->domain, i, &t ); +p2m_access_t a; + (void )p2m->get_entry (p2m, i, &t, &a, p2m_query,NULL );if (p2m_is_ram (t) ) { gfns[j] = i;
@@ -1112,7 +1116,8 @@ guest_physmap_mark_populate_on_demand(st/* Make sure all gpfns are unused */ for ( i =0 ; i < (1UL << order); i++ ) { - omfn =gfn_to_mfn_query (d, gfn + i, &ot); +p2m_access_t a; + omfn = p2m->get_entry (p2m, gfn + i, &ot, &a, p2m_query,NULL );if (p2m_is_ram (ot) ) { printk("%s: gfn_to_mfn returned type %d!\n",
xen/arch/x86/mm/p2m-pt.c +4 -2
@@ -1051,7 +1051,7 @@ void audit_p2m(struct p2m_domain *p2m, icontinue ; } - p2mfn =gfn_to_mfn_type_p2m (p2m, gfn, &type, p2m_query); + p2mfn =get_gfn_query (p2m->domain, gfn, &type);if ( strict_m2p &&mfn_x (p2mfn) != mfn ) { mpbad++;
@@ -1066,15 +1066,17 @@ void audit_p2m(struct p2m_domain *p2m, i* blow away the m2p entry. */ set_gpfn_from_mfn (mfn,INVALID_M2P_ENTRY ); } +put_gfn (p2m->domain, gfn);if ( test_linear && (gfn <= p2m->max_mapped_pfn) ) { - lp2mfn =mfn_x (gfn_to_mfn_type_p2m (p2m, gfn, &type, p2m_query)); + lp2mfn =mfn_x (get_gfn_query (p2m->domain, gfn, &type));if ( lp2mfn !=mfn_x (p2mfn) ) {P2M_PRINTK ("linear mismatch gfn %#lx -> mfn %#lx " "(!= mfn %#lx)\n" , gfn, lp2mfn,mfn_x (p2mfn)); } +put_gfn (p2m->domain, gfn); }// P2M_PRINTK("OK: mfn=%#lx, gfn=%#lx, p2mfn=%#lx, lp2mfn=%#lx\n",
xen/arch/x86/mm/p2m.c +20 -14
@@ -343,7 +343,6 @@ void p2m_teardown(struct p2m_domain *p2m#ifdef __x86_64__unsigned long gfn;p2m_type_t t; -p2m_access_t a;mfn_t mfn;#endif
@@ -353,13 +352,13 @@ void p2m_teardown(struct p2m_domain *p2m#ifdef __x86_64__for ( gfn=0 ; gfn < p2m->max_mapped_pfn; gfn++ ) { - mfn =gfn_to_mfn_type_p2m (p2m, gfn, &t, &a, p2m_query,NULL ); + mfn =get_gfn_query (d, gfn, &t);if (mfn_valid (mfn) && (t == p2m_ram_shared) ) {ASSERT (!p2m_is_nestedp2m (p2m));BUG_ON (mem_sharing_unshare_page (d, gfn,MEM_SHARING_DESTROY_GFN )); } - +put_gfn (d, gfn); }#endif
@@ -454,6 +453,7 @@ guest_physmap_add_entry(struct domain *dstruct p2m_domain *p2m =p2m_get_hostp2m (d);unsigned long i, ogfn;p2m_type_t ot; +p2m_access_t a;mfn_t omfn;int pod_count =0 ;int rc =0 ;
@@ -489,12 +489,13 @@ guest_physmap_add_entry(struct domain *d/* First, remove m->p mappings for existing p->m mappings */ for ( i =0 ; i < (1UL << page_order); i++ ) { - omfn =gfn_to_mfn_query (d, gfn + i, &ot); + omfn = p2m->get_entry (p2m, gfn + i, &ot, &a, p2m_query,NULL );if (p2m_is_grant (ot) ) {/* Really shouldn't be unmapping grant maps this way */ domain_crash (d);p2m_unlock (p2m); +return -EINVAL ; }else if (p2m_is_ram (ot) )
@@ -528,7 +529,7 @@ guest_physmap_add_entry(struct domain *d* address */ P2M_DEBUG ("aliased! mfn=%#lx, old gfn=%#lx, new gfn=%#lx\n" , mfn + i, ogfn, gfn + i); - omfn =gfn_to_mfn_query (d, ogfn, &ot); + omfn = p2m->get_entry (p2m, ogfn, &ot, &a, p2m_query,NULL );if (p2m_is_ram (ot) ) {ASSERT (mfn_valid (omfn));
@@ -577,6 +578,7 @@ guest_physmap_add_entry(struct domain *dp2m_type_t p2m_change_type (struct domain *d,unsigned long gfn,p2m_type_t ot,p2m_type_t nt) { +p2m_access_t a;p2m_type_t pt;mfn_t mfn;struct p2m_domain *p2m =p2m_get_hostp2m (d);
@@ -585,7 +587,7 @@ p2m_type_t p2m_change_type(struct domainp2m_lock (p2m); - mfn =gfn_to_mfn_query (d, gfn, &pt); + mfn = p2m->get_entry (p2m, gfn, &pt, &a, p2m_query,NULL );if ( pt == ot )set_p2m_entry (p2m, gfn, mfn,0 , nt, p2m->default_access);
@@ -600,6 +602,7 @@ void p2m_change_type_range(struct domain unsigned long start, unsigned long end, p2m_type_t ot, p2m_type_t nt) { + p2m_access_t a;p2m_type_t pt;unsigned long gfn;mfn_t mfn;
@@ -612,7 +615,7 @@ void p2m_change_type_range(struct domainfor ( gfn = start; gfn < end; gfn++ ) { - mfn =gfn_to_mfn_query (d, gfn, &pt); + mfn = p2m->get_entry (p2m, gfn, &pt, &a, p2m_query,NULL );if ( pt == ot )set_p2m_entry (p2m, gfn, mfn,0 , nt, p2m->default_access); }
@@ -629,6 +632,7 @@ int set_mmio_p2m_entry(struct domain *d, unsigned long gfn, mfn_t mfn) { int rc = 0; +p2m_access_t a;p2m_type_t ot;mfn_t omfn;struct p2m_domain *p2m =p2m_get_hostp2m (d);
@@ -637,7 +641,7 @@ set_mmio_p2m_entry(struct domain *d, unsreturn 0 ;p2m_lock (p2m); - omfn =gfn_to_mfn_query (d, gfn, &ot); + omfn = p2m->get_entry (p2m, gfn, &ot, &a, p2m_query,NULL );if (p2m_is_grant (ot) ) {p2m_unlock (p2m);
@@ -657,7 +661,7 @@ set_mmio_p2m_entry(struct domain *d, unsif (0 == rc )gdprintk (XENLOG_ERR ,"set_mmio_p2m_entry: set_p2m_entry failed! mfn=%08lx\n" , -mfn_x (gfn_to_mfn_query (d, gfn, &ot))); +mfn_x (p2m->get_entry (p2m, gfn, &ot, &a, p2m_query,NULL )));return rc; }
@@ -666,6 +670,7 @@ clear_mmio_p2m_entry(struct domain *d, u {int rc =0 ;mfn_t mfn; +p2m_access_t a;p2m_type_t t;struct p2m_domain *p2m =p2m_get_hostp2m (d);
@@ -673,7 +678,7 @@ clear_mmio_p2m_entry(struct domain *d, ureturn 0 ;p2m_lock (p2m); - mfn =gfn_to_mfn_query (d, gfn, &t); + mfn = p2m->get_entry (p2m, gfn, &t, &a, p2m_query,NULL );/* Do not use mfn_valid() here as it will usually fail for MMIO pages. */ if ( (INVALID_MFN ==mfn_x (mfn)) || (t != p2m_mmio_direct) )
@@ -696,6 +701,7 @@ set_shared_p2m_entry(struct domain *d, u {struct p2m_domain *p2m =p2m_get_hostp2m (d);int rc =0 ; +p2m_access_t a;p2m_type_t ot;mfn_t omfn;
@@ -703,7 +709,7 @@ set_shared_p2m_entry(struct domain *d, ureturn 0 ;p2m_lock (p2m); - omfn =gfn_to_mfn_query (p2m->domain, gfn, &ot); + omfn = p2m->get_entry (p2m, gfn, &ot, &a, p2m_query,NULL );/* At the moment we only allow p2m change if gfn has already been made * sharable first */ ASSERT (p2m_is_shared (ot));
@@ -717,7 +723,7 @@ set_shared_p2m_entry(struct domain *d, uif (0 == rc )gdprintk (XENLOG_ERR ,"set_shared_p2m_entry: set_p2m_entry failed! mfn=%08lx\n" , -mfn_x (gfn_to_mfn_query (d, gfn, &ot))); +mfn_x (p2m->get_entry (p2m, gfn, &ot, &a, p2m_query,NULL )));return rc; }
@@ -1168,7 +1174,7 @@ int p2m_set_mem_access(struct domain *d, {struct p2m_domain *p2m =p2m_get_hostp2m (d);unsigned long pfn; -p2m_access_t a; +p2m_access_t a, _a;p2m_type_t t;mfn_t mfn;int rc =0 ;
@@ -1202,7 +1208,7 @@ int p2m_set_mem_access(struct domain *d,p2m_lock (p2m);for ( pfn = start_pfn; pfn < start_pfn + nr; pfn++ ) { - mfn =gfn_to_mfn_query (d, pfn, &t); + mfn = p2m->get_entry (p2m, pfn, &t, &_a, p2m_query,NULL );if ( p2m->set_entry (p2m, pfn, mfn,PAGE_ORDER_4K , t, a) ==0 ) { rc = -ENOMEM ;
xen/arch/x86/mm/shadow/common.c +4 -2
@@ -3676,7 +3676,7 @@ int shadow_track_dirty_vram(struct domai/* Iterate over VRAM to track dirty bits. */ for ( i =0 ; i < nr; i++ ) { -mfn_t mfn =gfn_to_mfn_query (d, begin_pfn + i, &t); +mfn_t mfn =get_gfn_query (d, begin_pfn + i, &t);struct page_info *page;int dirty =0 ;paddr_t sl1ma = dirty_vram->sl1ma[i];
@@ -3741,6 +3741,8 @@ int shadow_track_dirty_vram(struct domai } } +put_gfn (d, begin_pfn + i); +if ( dirty ) { dirty_vram->dirty_bitmap[i /8 ] |=1 << (i %8 );
@@ -3761,7 +3763,7 @@ int shadow_track_dirty_vram(struct domai/* was clean for more than two seconds, try to disable guest * write access */ for ( i = begin_pfn; i < end_pfn; i++ ) { -mfn_t mfn =gfn_to_mfn_query (d, i, &t); +mfn_t mfn =get_gfn_query_unlocked (d, i, &t);if (mfn_x (mfn) !=INVALID_MFN ) flush_tlb |=sh_remove_write_access (d->vcpu[0 ], mfn,1 ,0 ); }
xen/arch/x86/mm/shadow/multi.c +61 -24
330 lines not on this page · raw message
xen/arch/x86/mm/shadow/types.h +5 -5
@@ -191,11 +191,11 @@ static inline shadow_l4e_t shadow_l4e_fr })#endif -/* Override gfn_to_mfn to work with gfn_t */ -#undef gfn_to_mfn_query -#define gfn_to_mfn_query (d, g, t) gfn_to_mfn_type((d), gfn_x(g), (t), p2m_query) -#undef gfn_to_mfn_guest -#define gfn_to_mfn_guest (d, g, t) gfn_to_mfn_type((d), gfn_x(g), (t), p2m_guest) +/* Override get_gfn to work with gfn_t */ +#undef get_gfn_query +#define get_gfn_query (d, g, t) get_gfn_type((d), gfn_x(g), (t), p2m_query) +#undef get_gfn_guest +#define get_gfn_guest (d, g, t) get_gfn_type((d), gfn_x(g), (t), p2m_guest)/* The shadow types needed for the various levels. */
xen/arch/x86/physdev.c +7 -1
@@ -297,16 +297,20 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_Hbreak ; ret = -EINVAL ; - mfn =gmfn_to_mfn (current->domain, info.gmfn); + mfn =get_gfn_untyped (current->domain, info.gmfn);if ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), v->domain, PGT_writable_page) ) + { +put_gfn (current->domain, info.gmfn);break ; + }if (cmpxchg (&v->domain->arch.pv_domain.pirq_eoi_map_mfn,0 , mfn) !=0 ) {put_page_and_type (mfn_to_page (mfn)); +put_gfn (current->domain, info.gmfn); ret = -EBUSY ;break ; }
@@ -316,10 +320,12 @@ ret_t do_physdev_op(int cmd, XEN_GUEST_H { v->domain->arch.pv_domain.pirq_eoi_map_mfn =0 ;put_page_and_type (mfn_to_page (mfn)); +put_gfn (current->domain, info.gmfn); ret = -ENOSPC ;break ; } +put_gfn (current->domain, info.gmfn); ret =0 ;break ; }
xen/arch/x86/traps.c +14 -5
@@ -673,11 +673,12 @@ int wrmsr_hypervisor_regs(uint32_t idx,return 0 ; } - mfn =gmfn_to_mfn (d, gmfn); + mfn =get_gfn_untyped (d, gmfn);if ( !mfn_valid (mfn) || !get_page_and_type (mfn_to_page (mfn), d, PGT_writable_page) ) { +put_gfn (d, gmfn);gdprintk (XENLOG_WARNING ,"Bad GMFN %lx (MFN %lx) to MSR %08x\n" , gmfn, mfn, base + idx);
@@ -689,6 +690,7 @@ int wrmsr_hypervisor_regs(uint32_t idx,unmap_domain_page (hypercall_page);put_page_and_type (mfn_to_page (mfn)); +put_gfn (d, gmfn);break ; }
@@ -2347,18 +2349,25 @@ static int emulate_privileged_op(structarch_set_cr2 (v, *reg);break ; -case 3 :/* Write CR3 */ +case 3 : {/* Write CR3 */ +unsigned long mfn, gfn;domain_lock (v->domain);if ( !is_pv_32on64_vcpu (v) ) - rc =new_guest_cr3 (gmfn_to_mfn (v->domain,xen_cr3_to_pfn (*reg))); + { + gfn =xen_cr3_to_pfn (*reg); #ifdef CONFIG_COMPAT - else - rc =new_guest_cr3 (gmfn_to_mfn (v->domain,compat_cr3_to_pfn (*reg))); + }else { + gfn =compat_cr3_to_pfn (*reg);#endif + } + mfn =get_gfn_untyped (v->domain, gfn); + rc =new_guest_cr3 (mfn); +put_gfn (v->domain, gfn);domain_unlock (v->domain);if ( rc ==0 )/* not okay */ goto fail;break ; + }case 4 :/* Write CR4 */ v->arch.pv_vcpu.ctrlreg[4 ] =pv_guest_cr4_fixup (v, *reg);
xen/common/grant_table.c +24 -6
@@ -110,7 +110,7 @@ static unsigned inline int max_nr_maptra#define gfn_to_mfn_private (_d, _gfn) ({ \ p2m_type_t __p2mt; \ unsigned long __x; \ - __x = mfn_x(gfn_to_mfn_unshare((_d), (_gfn), &__p2mt)); \ + __x = mfn_x(get_gfn_unshare((_d), (_gfn), &__p2mt)); \ BUG_ON(p2m_is_shared(__p2mt));/* XXX fixme */ \if ( !p2m_is_valid (__p2mt) ) \ __x =INVALID_MFN ; \
@@ -150,10 +150,10 @@ static int __get_paged_frame(unsigned lomfn_t mfn;if ( readonly ) - mfn =gfn_to_mfn (rd, gfn, &p2mt); + mfn =get_gfn (rd, gfn, &p2mt);else { - mfn =gfn_to_mfn_unshare (rd, gfn, &p2mt); + mfn =get_gfn_unshare (rd, gfn, &p2mt);BUG_ON (p2m_is_shared (p2mt));/* XXX Here, and above in gfn_to_mfn_private, need to handle * XXX failure to unshare. */
@@ -164,14 +164,16 @@ static int __get_paged_frame(unsigned loif (p2m_is_paging (p2mt) ) {p2m_mem_paging_populate (rd, gfn); +put_gfn (rd, gfn); rc = GNTST_eagain; } }else { +put_gfn (rd, gfn); *frame =INVALID_MFN ; rc = GNTST_bad_page; }#else - *frame = readonly ?gmfn_to_mfn (rd, gfn) :gfn_to_mfn_private (rd, gfn); + *frame = readonly ?get_gfn_untyped (rd, gfn) :gfn_to_mfn_private (rd, gfn);#endif return rc;
@@ -468,13 +470,14 @@ __gnttab_map_grant_ref(struct domain *ld, *rd, *owner;struct vcpu *led;int handle; +unsigned long gfn =INVALID_GFN ;unsigned long frame =0 , nr_gets =0 ;struct page_info *pg;int rc = GNTST_okay;u32 old_pin;u32 act_pin;unsigned int cache_flags; -struct active_grant_entry *act; +struct active_grant_entry *act =NULL ;struct grant_mapping *mt;grant_entry_v1_t *sha1;grant_entry_v2_t *sha2;
@@ -565,7 +568,6 @@ __gnttab_map_grant_ref(if ( !act->pin ) { -unsigned long gfn;unsigned long frame; gfn = sha1 ? sha1->frame : sha2->full_page.frame;
@@ -698,6 +700,7 @@ __gnttab_map_grant_ref( op->handle = handle; op->status = GNTST_okay; +put_gfn (rd, gfn);rcu_unlock_domain (rd);return ;
@@ -735,6 +738,8 @@ __gnttab_map_grant_ref(gnttab_clear_flag (_GTF_reading, status); unlock_out: +if ( gfn !=INVALID_GFN ) +put_gfn (rd, gfn);spin_unlock (&rd->grant_table->lock); op->status = rc;put_maptrack_handle (ld->grant_table, handle);
@@ -1475,6 +1480,7 @@ gnttab_transfer(/* Check the passed page frame for basic validity. */ if (unlikely (!mfn_valid (mfn)) ) { +put_gfn (d, gop.mfn);gdprintk (XENLOG_INFO ,"gnttab_transfer: out-of-range %lx\n" , (unsigned long )gop.mfn); gop.status = GNTST_bad_page;
@@ -1484,6 +1490,7 @@ gnttab_transfer( page =mfn_to_page (mfn);if (unlikely (is_xen_heap_page (page)) ) { +put_gfn (d, gop.mfn);gdprintk (XENLOG_INFO ,"gnttab_transfer: xen frame %lx\n" , (unsigned long )gop.mfn); gop.status = GNTST_bad_page;
@@ -1492,6 +1499,7 @@ gnttab_transfer(if (steal_page (d, page,0 ) <0 ) { +put_gfn (d, gop.mfn); gop.status = GNTST_bad_page; goto copyback; }
@@ -1504,6 +1512,7 @@ gnttab_transfer(/* Find the target domain. */ if (unlikely ((e =rcu_lock_domain_by_id (gop.domid)) ==NULL ) ) { +put_gfn (d, gop.mfn);gdprintk (XENLOG_INFO ,"gnttab_transfer: can't find domain %d\n" , gop.domid); page->count_info &= ~(PGC_count_mask|PGC_allocated);
@@ -1514,6 +1523,7 @@ gnttab_transfer(if (xsm_grant_transfer (d, e) ) { +put_gfn (d, gop.mfn); gop.status = GNTST_permission_denied; unlock_and_copyback:rcu_unlock_domain (e);
@@ -1566,6 +1576,7 @@ gnttab_transfer( e->tot_pages, e->max_pages, gop.ref, e->is_dying);spin_unlock (&e->page_alloc_lock);rcu_unlock_domain (e); +put_gfn (d, gop.mfn); page->count_info &= ~(PGC_count_mask|PGC_allocated);free_domheap_page (page); gop.status = GNTST_general_error;
@@ -1579,6 +1590,7 @@ gnttab_transfer(page_set_owner (page, e);spin_unlock (&e->page_alloc_lock); +put_gfn (d, gop.mfn);TRACE_1D (TRC_MEM_PAGE_GRANT_TRANSFER , e->domain_id);
@@ -1850,6 +1862,8 @@ __acquire_grant_for_copy( { gfn = sha1->frame; rc =__get_paged_frame (gfn, &grant_frame, readonly, rd); +/* We drop this immediately per the comments at the top */ +put_gfn (rd, gfn);if ( rc != GNTST_okay ) goto unlock_out; act->gfn = gfn;
@@ -1862,6 +1876,7 @@ __acquire_grant_for_copy( { gfn = sha2->full_page.frame; rc =__get_paged_frame (gfn, &grant_frame, readonly, rd); +put_gfn (rd, gfn);if ( rc != GNTST_okay ) goto unlock_out; act->gfn = gfn;
@@ -1874,6 +1889,7 @@ __acquire_grant_for_copy( { gfn = sha2->sub_page.frame; rc =__get_paged_frame (gfn, &grant_frame, readonly, rd); +put_gfn (rd, gfn);if ( rc != GNTST_okay ) goto unlock_out; act->gfn = gfn;
@@ -1973,6 +1989,7 @@ __gnttab_copy( {#ifdef CONFIG_X86 rc =__get_paged_frame (op->source.u.gmfn, &s_frame,1 , sd); +put_gfn (sd, op->source.u.gmfn);if ( rc != GNTST_okay ) goto error_out;#else
@@ -2012,6 +2029,7 @@ __gnttab_copy( {#ifdef CONFIG_X86 rc =__get_paged_frame (op->dest.u.gmfn, &d_frame,0 , dd); +put_gfn (dd, op->dest.u.gmfn);if ( rc != GNTST_okay ) goto error_out;#else
xen/common/memory.c +11 -2
@@ -162,11 +162,12 @@ int guest_remove_page(struct domain *d,unsigned long mfn;#ifdef CONFIG_X86 - mfn =mfn_x (gfn_to_mfn (d, gmfn, &p2mt)); + mfn =mfn_x (get_gfn (d, gmfn, &p2mt));if (unlikely (p2m_is_paging (p2mt)) ) {guest_physmap_remove_page (d, gmfn, mfn,0 );p2m_mem_paging_drop_page (d, gmfn); +put_gfn (d, gmfn);return 1 ; }#else
@@ -174,6 +175,7 @@ int guest_remove_page(struct domain *d,#endif if (unlikely (!mfn_valid (mfn)) ) { +put_gfn (d, gmfn);gdprintk (XENLOG_INFO ,"Domain %u page number %lx invalid\n" , d->domain_id, gmfn);return 0 ;
@@ -187,12 +189,14 @@ int guest_remove_page(struct domain *d, {put_page_and_type (page);guest_physmap_remove_page (d, gmfn, mfn,0 ); +put_gfn (d, gmfn);return 1 ; }#endif /* CONFIG_X86 */ if (unlikely (!get_page (page, d)) ) { +put_gfn (d, gmfn);gdprintk (XENLOG_INFO ,"Bad page free for domain %u\n" , d->domain_id);return 0 ; }
@@ -204,6 +208,7 @@ int guest_remove_page(struct domain *d,put_page (page);guest_physmap_remove_page (d, gmfn, mfn,0 ); +put_gfn (d, gmfn);put_page (page);
@@ -363,9 +368,10 @@ static long memory_exchange(XEN_GUEST_HAp2m_type_t p2mt;/* Shared pages cannot be exchanged */ - mfn =mfn_x (gfn_to_mfn_unshare (d, gmfn + k, &p2mt)); + mfn =mfn_x (get_gfn_unshare (d, gmfn + k, &p2mt));if (p2m_is_shared (p2mt) ) { +put_gfn (d, gmfn + k); rc = -ENOMEM ; goto fail; }
@@ -374,6 +380,7 @@ static long memory_exchange(XEN_GUEST_HA#endif if (unlikely (!mfn_valid (mfn)) ) { +put_gfn (d, gmfn + k); rc = -EINVAL ; goto fail; }
@@ -382,11 +389,13 @@ static long memory_exchange(XEN_GUEST_HAif (unlikely (steal_page (d, page, MEMF_no_refcount)) ) { +put_gfn (d, gmfn + k); rc = -EINVAL ; goto fail; }page_list_add (page, &in_chunk_list); +put_gfn (d, gmfn + k); } }
xen/common/tmem_xen.c +14 -7
@@ -109,22 +109,28 @@ static inline void *cli_get_page(tmem_clstruct page_info *page;int ret; - cli_mfn =mfn_x (gfn_to_mfn (current->domain, cmfn, &t)); + cli_mfn =mfn_x (get_gfn (current->domain, cmfn, &t));if ( t != p2m_ram_rw || !mfn_valid (cli_mfn) ) + { +put_gfn (current->domain, (unsigned long ) cmfn);return NULL ; + } page =mfn_to_page (cli_mfn);if ( cli_write ) ret =get_page_and_type (page, current->domain, PGT_writable_page);else ret =get_page (page, current->domain);if ( !ret ) + { +put_gfn (current->domain, (unsigned long ) cmfn);return NULL ; + } *pcli_mfn = cli_mfn; *pcli_pfp = (pfp_t *)page;return map_domain_page (cli_mfn); } -static inline void cli_put_page (void *cli_va,pfp_t *cli_pfp, +static inline void cli_put_page (tmem_cli_mfn_t cmfn,void *cli_va,pfp_t *cli_pfp,unsigned long cli_mfn,bool_t mark_dirty) {if ( mark_dirty )
@@ -135,6 +141,7 @@ static inline void cli_put_page(void *cl else put_page((struct page_info *)cli_pfp);unmap_domain_page (cli_va); + put_gfn(current->domain, (unsigned long) cmfn); }#endif
@@ -169,7 +176,7 @@ EXPORT int tmh_copy_from_client(pfp_t *p (pfn_offset+len <= PAGE_SIZE) )memcpy ((char *)tmem_va+tmem_offset,(char *)cli_va+pfn_offset,len);if ( !tmemc ) -cli_put_page (cli_va, cli_pfp, cli_mfn,0 ); +cli_put_page (cmfn, cli_va, cli_pfp, cli_mfn,0 );unmap_domain_page (tmem_va);return 1 ; }
@@ -197,7 +204,7 @@ EXPORT int tmh_compress_from_client(tmemASSERT (ret ==LZO_E_OK ); *out_va = dmem;if ( !tmemc ) -cli_put_page (cli_va, cli_pfp, cli_mfn,0 ); +cli_put_page (cmfn, cli_va, cli_pfp, cli_mfn,0 );unmap_domain_page (cli_va);return 1 ; }
@@ -225,7 +232,7 @@ EXPORT int tmh_copy_to_client(tmem_cli_mmemcpy ((char *)cli_va+pfn_offset,(char *)tmem_va+tmem_offset,len);unmap_domain_page (tmem_va);if ( !tmemc ) -cli_put_page (cli_va, cli_pfp, cli_mfn,1 ); +cli_put_page (cmfn, cli_va, cli_pfp, cli_mfn,1 );mb ();return 1 ; }
@@ -249,7 +256,7 @@ EXPORT int tmh_decompress_to_client(tmemASSERT (ret ==LZO_E_OK );ASSERT (out_len ==PAGE_SIZE );if ( !tmemc ) -cli_put_page (cli_va, cli_pfp, cli_mfn,1 ); +cli_put_page (cmfn, cli_va, cli_pfp, cli_mfn,1 );mb ();return 1 ; }
@@ -271,7 +278,7 @@ EXPORT int tmh_copy_tze_to_client(tmem_cmemcpy ((char *)cli_va,(char *)tmem_va,len);if ( len <PAGE_SIZE )memset ((char *)cli_va+len,0 ,PAGE_SIZE -len); -cli_put_page (cli_va, cli_pfp, cli_mfn,1 ); +cli_put_page (cmfn, cli_va, cli_pfp, cli_mfn,1 );mb ();return 1 ; }
xen/include/asm-ia64/mm.h +2 -0
@@ -549,6 +549,8 @@ extern u64 translate_domain_pte(u64 ptev#define gmfn_to_mfn (_d, gpfn) \ gmfn_to_mfn_foreign((_d), (gpfn)) +#define put_gfn (d, g) ((void)0) +#define __gpfn_invalid (_d, gpfn) \ (lookup_domain_mpa((_d), ((gpfn)<<PAGE_SHIFT), NULL) == INVALID_MFN)
xen/include/asm-x86/guest_pt.h +3 -3
@@ -51,9 +51,9 @@ gfn_to_paddr(gfn_t gfn)return ((paddr_t )gfn_x (gfn)) <<PAGE_SHIFT ; } -/* Override gfn_to_mfn to work with gfn_t */ -#undef gfn_to_mfn -#define gfn_to_mfn (d, g, t) gfn_to_mfn_type((d), gfn_x(g), (t), p2m_alloc) +/* Override get_gfn to work with gfn_t */ +#undef get_gfn +#define get_gfn (d, g, t) get_gfn_type((d), gfn_x(g), (t), p2m_alloc)/* Types of the guest's page tables and access functions for them */
xen/include/asm-x86/hvm/hvm.h +4 -1
@@ -394,7 +394,10 @@ int hvm_virtual_to_linear_addr(void *hvm_map_guest_frame_rw (unsigned long gfn);void *hvm_map_guest_frame_ro (unsigned long gfn); -void hvm_unmap_guest_frame (void *p); +/* We pass back either the guest virtual or physical frame mapped, +* in order to drop any locks/refcounts we may have had on p2m +* entries or underlying mfn's while using the map */ +void hvm_unmap_guest_frame (void *p,unsigned long addr,int is_va);static inline void hvm_set_info_guest (struct vcpu *v) {
xen/include/asm-x86/hvm/vmx/vvmx.h +1 -0
@@ -25,6 +25,7 @@struct nestedvmx {paddr_t vmxon_region_pa; +unsigned long iobitmap_gfn[2 ];void *iobitmap[2 ];/* map (va) of L1 guest I/O bitmap */ /* deferred nested interrupt */ struct {
xen/include/asm-x86/p2m.h +28 -10
@@ -305,9 +305,12 @@ struct p2m_domain *p2m_get_p2m(struct vc#define p2m_get_pagetable (p2m) ((p2m)->phys_table) +/**** p2m query accessors. After calling any of the variants below, you +* need to call put_gfn(domain, gfn). If you don't, you'll lock the +* hypervisor. ****/ /* Read a particular P2M table, mapping pages as we go. Most callers -* should _not_ call this directly; use the other gfn_to_mfn_* functions +* should _not_ call this directly; use the other get_gfn* functions * below unless you know you want to walk a p2m that isn't a domain's * main one. * If the lookup succeeds, the return value is != INVALID_MFN and
@@ -318,7 +321,7 @@ mfn_t gfn_to_mfn_type_p2m(struct p2m_dom unsigned int *page_order);/* General conversion function from gfn to mfn */ -static inline mfn_t gfn_to_mfn_type (struct domain *d, +static inline mfn_t get_gfn_type (struct domain *d,unsigned long gfn,p2m_type_t *t,p2m_query_t q) {
@@ -327,25 +330,39 @@ static inline mfn_t gfn_to_mfn_type(stru }/* Syntactic sugar: most callers will use one of these. -* N.B. gfn_to_mfn_query() is the _only_ one guaranteed not to take the +* N.B. get_gfn_query() is the _only_ one guaranteed not to take the * p2m lock; none of the others can be called with the p2m or paging * lock held. */ -#define gfn_to_mfn (d, g, t) gfn_to_mfn_type((d), (g), (t), p2m_alloc) -#define gfn_to_mfn_query (d, g, t) gfn_to_mfn_type((d), (g), (t), p2m_query) -#define gfn_to_mfn_guest (d, g, t) gfn_to_mfn_type((d), (g), (t), p2m_guest) -#define gfn_to_mfn_unshare (d, g, t) gfn_to_mfn_type((d), (g), (t), p2m_unshare) +#define get_gfn (d, g, t) get_gfn_type((d), (g), (t), p2m_alloc) +#define get_gfn_query (d, g, t) get_gfn_type((d), (g), (t), p2m_query) +#define get_gfn_guest (d, g, t) get_gfn_type((d), (g), (t), p2m_guest) +#define get_gfn_unshare (d, g, t) get_gfn_type((d), (g), (t), p2m_unshare)/* Compatibility function exporting the old untyped interface */ -static inline unsigned long gmfn_to_mfn (struct domain *d,unsigned long gpfn) +static inline unsigned long get_gfn_untyped (struct domain *d,unsigned long gpfn) {mfn_t mfn;p2m_type_t t; - mfn =gfn_to_mfn (d, gpfn, &t); + mfn =get_gfn (d, gpfn, &t);if (p2m_is_valid (t) )return mfn_x (mfn);return INVALID_MFN ; } +/* This is a noop for now. */ +static inline void put_gfn (struct domain *d,unsigned long gfn) +{ +} + +/* These are identical for now. The intent is to have the caller not worry +* about put_gfn. To only be used in printk's, crash situations, or to +* peek at a type. You're not holding the p2m entry exclsively after calling +* this. */ +#define get_gfn_unlocked (d, g, t) get_gfn_type((d), (g), (t), p2m_alloc) +#define get_gfn_query_unlocked (d, g, t) get_gfn_type((d), (g), (t), p2m_query) +#define get_gfn_guest_unlocked (d, g, t) get_gfn_type((d), (g), (t), p2m_guest) +#define get_gfn_unshare_unlocked (d, g, t) get_gfn_type((d), (g), (t), p2m_unshare) +/* General conversion function from mfn to gfn */ static inline unsigned long mfn_to_gfn (struct domain *d,mfn_t mfn)
Lines of this file are not shown. View the raw message.
The rest of this diff is not shown. View the raw message.
Part of the diff is not shown here. View the raw message.
Re: [PATCH 5 of 5] Modify naming of queries into the p2m
Hi,
This mostly looks good; I have a few comments on the detail below. If those are addressed I hope I can apply the next version.
Any other maintainers want to object to this renaming?
Quoted 9 lines · Andres Lagar-Cavilla
xen/arch/x86/hvm/hvm.c +1
-0@@ -1182,6 +1195,7 @@ int hvm_hap_nested_page_fault(unsigned lmfn_t mfn;struct vcpu *v = current;struct p2m_domain *p2m; +int rc;/* On Nested Virtualization, walk the guest page table. * If this succeeds, all is fine.
xen/arch/x86/hvm/hvm.c int hvm_hap_nested_page_fault(unsigned l · line 1273
Quoted 7 lines
xen/arch/x86/hvm/hvm.c +2
-2@@ -1255,8 +1269,8 @@ int hvm_hap_nested_page_fault(unsigned lif ( violation ) {p2m_mem_access_check (gpa, gla_valid, gla, access_r, access_w, access_x); - -return 1 ; + rc =1 ; + goto out_put_gfn;
Shouldn't this patch be changing the call to gfn_to_mfn_type_p2m() just above here to a get_gfn_*() call too?
Quoted 19 lines
xen/arch/x86/hvm/hvm.c +16
-1-void hvm_unmap_guest_frame (void *p) +void hvm_unmap_guest_frame (void *p,unsigned long addr,int is_va) { +/* We enter this function with a map obtained in __hvm_map_guest_frame. +* This map performed a p2m query that locked the gfn entry and got +* a ref on the mfn. Must undo */ if ( p ) + { +unsigned long gfn =INVALID_GFN ; + +if ( is_va ) + { +if ( addr ) + { +uint32_t pfec = PFEC_page_present; + gfn =paging_gva_to_gfn (current, addr, &pfec); + }else { + gfn =INVALID_GFN ; + }
xen/arch/x86/hvm/hvm.c void *hvm_map_guest_frame_ro(unsigned lo · line 1862
Quoted 10 lines
xen/arch/x86/hvm/hvm.c +8
-0+ }else { + gfn = addr; + } + +if ( gfn !=INVALID_GFN ) +put_gfn (current->domain, gfn); +unmap_domain_page (p); + } }
This is a pretty wierd-looking function now - I think it would be better just to make all callers have to remember the GFN. In fact, since a guest VCPU can change its pagetables at any time, it's not safe to use paging_gva_to_gfn() to regenerate the GFN from a VA.
Also, IIRC the nested-SVM code keeps the hvm_map mapping around for quite a long time so in fact this unmap-and-put-gfn may not be the right interface. Maybe the caller should put_gfn() explicitly.
Quoted 33 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/hvm/svm/svm.cxen/arch/x86/hvm/svm/svm.c +4
-2@@ -244,9 +244,10 @@ static int svm_vmcb_restore(struct vcpu {if ( c->cr0 &X86_CR0_PG ) { - mfn =mfn_x (gfn_to_mfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) || !get_page (mfn_to_page (mfn), v->domain) ) { +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT );gdprintk (XENLOG_ERR ,"Invalid CR3 value=0x%" PRIx64"\n" , c->cr3);return -EINVAL ;@@ -257,6 +258,8 @@ static int svm_vmcb_restore(struct vcpuput_page (pagetable_get_page (v->arch.guest_table)); v->arch.guest_table =pagetable_from_pfn (mfn); +if ( c->cr0 &X86_CR0_PG ) +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT ); } v->arch.hvm_vcpu.guest_cr[0 ] = c->cr0 |X86_CR0_ET ;@@ -1144,7 +1147,6 @@ static void svm_do_nested_pgfault(structunsigned long gfn = gpa >>PAGE_SHIFT ;mfn_t mfn;p2m_type_t p2mt; -p2m_access_t p2ma;struct p2m_domain *p2m =NULL ; ret =hvm_hap_nested_page_fault (gpa,0 , ~0ul ,0 ,0 ,0 ,0 );
xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1167
Quoted 6 lines
xen/arch/x86/hvm/svm/svm.c +2
-1@@ -1161,7 +1163,8 @@ static void svm_do_nested_pgfault(struct p2m =p2m_get_p2m (v); _d.gpa = gpa; _d.qualification =0 ; - _d.mfn =mfn_x (gfn_to_mfn_type_p2m (p2m, gfn, &_d.p2mt, &p2ma, p2m_query,NULL )); + mfn =get_gfn_query_unlocked (p2m->domain, gfn, &_d.p2mt); + _d.mfn =mfn_x (mfn);
Ah - this is not quite the same thing. This lookup uses a specific p2m table (possibly a nested-p2m table reflecting the fact the guest is running in nested SVM mode) so it can't be replaced by a call that just takes the domain pointer (and will internally use the domain's master p2m table).
The naming of 'p2m_get_p2m()' is particularly unhelpful here, I realise. It fetches the running p2m, i.e. the one that hardware sees as an NPT table; almost everywhere else uses p2m_get_hostp2m(), which fetches the master p2m. But in general when you see the gfn_to_mfn_*_p2m() functions, that take an explicit p2m pointer, you need to be careful.
xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1187
Quoted 9 lines
xen/arch/x86/hvm/svm/svm.c +1
-1__trace_var (TRC_HVM_NPF ,0 ,sizeof (_d), &_d); }@@ -1181,7 +1184,7 @@ static void svm_do_nested_pgfault(structif ( p2m ==NULL ) p2m =p2m_get_p2m (v);/* Everything else is an error. */ - mfn =gfn_to_mfn_type_p2m (p2m, gfn, &p2mt, &p2ma, p2m_guest,NULL ); + mfn =get_gfn_guest_unlocked (p2m->domain, gfn, &p2mt);
Likewise here.
Quoted 10 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/guest_walk.cxen/arch/x86/mm/guest_walk.c +2
-0@@ -86,30 +86,33 @@ static uint32_t set_ad_bits(void *guest_return 0 ; } +/* If the map is non-NULL, we leave this function having +* called get_gfn, you need to call put_gfn. */ static inline void *map_domain_gfn(struct p2m_domain *p2m,
xen/arch/x86/mm/guest_walk.c static uint32_t set_ad_bits(void *guest_ · line 98
Quoted 10 lines
xen/arch/x86/mm/guest_walk.c +1
-3gfn_t gfn, mfn_t *mfn, p2m_type_t *p2mt, uint32_t *rc) { - p2m_access_t a; -/* Translate the gfn, unsharing if shared */ - *mfn =gfn_to_mfn_type_p2m (p2m,gfn_x (gfn), p2mt, &a, p2m_unshare,NULL ); + *mfn =get_gfn_unshare (p2m->domain,gfn_x (gfn), p2mt);
Here's another case where we need to handle the nested-hap path; the p2m pointer here must be propagated into the lookup function.
Quoted 21 lines
xen/arch/x86/mm/guest_walk.c +3
-0if (p2m_is_paging (*p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain,gfn_x (gfn)); +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_PAGED;return NULL ; }if (p2m_is_shared (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_SHARED;return NULL ; }if ( !p2m_is_ram (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc |= _PAGE_PRESENT;return NULL ; }
xen/arch/x86/mm/guest_walk.c static inline void *map_domain_gfn(struc · line 128
Quoted 6 lines
xen/arch/x86/mm/guest_walk.c +3
-0@@ -120,6 +123,9 @@ static inline void *map_domain_gfn(struc/* Walk the guest pagetables, after the manner of a hardware walker. */ +/* Because the walk is essentially random, it can cause a deadlock +* warning in the p2m locking code. Highly unlikely this is an actual +* deadlock, because who would walk page table in the opposite order? */
Linear pagetables make this sort of thing more likely, especially if they're used by one process to update another process's mappings.
If this is a problem, maybe we'll need to avoid the deadlock either by allowing multiple lock-holders in the p2m or by rearranging the a/d writeback soit does another p2m lookup -- I'd rather not do that, though, since even on 64-bit it will add a lot of memory accesses.
I'm happy to take a version of this big switchover patch that doesn't solve this problem, though. It can be sorted out in a separate patch.
xen/arch/x86/mm/hap/nested_hap.c nestedhap_walk_L0_p2m(struct p2m_domain · line 153
Quoted 12 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/hap/nested_hap.cxen/arch/x86/mm/hap/nested_hap.c +1
-0@@ -146,22 +146,29 @@ nestedhap_walk_L0_p2m(struct p2m_domainmfn_t mfn;p2m_type_t p2mt;p2m_access_t p2ma; +int rc;/* walk L0 P2M table */ mfn =gfn_to_mfn_type_p2m (p2m, L1_gpa >>PAGE_SHIFT , &p2mt, &p2ma, p2m_query, page_order);
Again, are we not changing this function's name?
Cheers,
Tim.
Re: [PATCH 5 of 5] Modify naming of queries into the p2m
Ok, thanks. I'll try to address those comments asap in a new version.
Off-the-bat:
- sharing deadlock: I can arrange it so that shr_lock is always taken after get_gfn. That would still require either disabling deadlock detection or moving shr_lock down the deadlock order. Which one is preferable?
The real solution that I have queued is removing the global hash table, and locking each shared page individually using PGT_locked.
- get_mfn: the goal is additional belts-and-braces. It doesn't require an extra argument. Put_gfn will put the mfn that sits in the p2m entry at the time of put. A few tricks are needed to handle remove_page, sharing, etc.
Works for me with windows and linux hvm guests (so far)
- I can rename gfn_to_mfn_type_p2m to get_gfn_type_access (since that's what the remaining uses are for)
Thanks for your feedback on the nested p2m functions, which I quite don't get yet.
Andres
Quoted 223 lines
Hi,
This mostly looks good; I have a few comments on the detail below. If those are addressed I hope I can apply the next version.
Any other maintainers want to object to this renaming?
Quoted 9 lines · Andres Lagar-Cavilla
xen/arch/x86/hvm/hvm.c +1
-0@@ -1182,6 +1195,7 @@ int hvm_hap_nested_page_fault(unsigned lmfn_t mfn;struct vcpu *v = current;struct p2m_domain *p2m; +int rc;/* On Nested Virtualization, walk the guest page table. * If this succeeds, all is fine. xen/arch/x86/hvm/hvm.c int hvm_hap_nested_page_fault(unsigned l · line 1274
Quoted 8 lines
xen/arch/x86/hvm/hvm.c +2
-2@@ -1255,8 +1269,8 @@ int hvm_hap_nested_page_fault(unsigned lif ( violation ) {p2m_mem_access_check (gpa, gla_valid, gla, access_r, access_w, access_x); - -return 1 ; + rc =1 ; + goto out_put_gfn;Shouldn't this patch be changing the call to
gfn_to_mfn_type_p2m()just above here to a get_gfn_*() call too?Quoted 20 lines
xen/arch/x86/hvm/hvm.c +2
-1-void hvm_unmap_guest_frame (void *p) +void hvm_unmap_guest_frame (void *p,unsigned long addr,int is_va) { +/* We enter this function with a map obtained in
__hvm_map_guest_frame.xen/arch/x86/hvm/hvm.c +14
-0+* This map performed a p2m query that locked the gfn entry and got +* a ref on the mfn. Must undo */ if ( p ) + { +unsigned long gfn =INVALID_GFN ; + +if ( is_va ) + { +if ( addr ) + { +uint32_t pfec = PFEC_page_present; + gfn =paging_gva_to_gfn (current, addr, &pfec); + }else { + gfn =INVALID_GFN ; + }xen/arch/x86/hvm/hvm.c void *hvm_map_guest_frame_ro(unsigned lo · line 1862
Quoted 10 lines
xen/arch/x86/hvm/hvm.c +8
-0+ }else { + gfn = addr; + } + +if ( gfn !=INVALID_GFN ) +put_gfn (current->domain, gfn); +unmap_domain_page (p); + } }This is a pretty wierd-looking function now - I think it would be better just to make all callers have to remember the GFN. In fact, since a guest VCPU can change its pagetables at any time, it's not safe to use
paging_gva_to_gfn()to regenerate the GFN from a VA.Also, IIRC the nested-SVM code keeps the
hvm_mapmapping around for quite a long time so in fact this unmap-and-put-gfn may not be the right interface. Maybe the caller shouldput_gfn()explicitly.Quoted 36 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/hvm/svm/svm.cxen/arch/x86/hvm/svm/svm.c +2
-1@@ -244,9 +244,10 @@ static int svm_vmcb_restore(struct vcpu {if ( c->cr0 &X86_CR0_PG ) { - mfn =mfn_x (gfn_to_mfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) || !get_page (mfn_to_page (mfn), v->domain) ) { +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT );gdprintk(XENLOG_ERR, "Invalid CR3 value=0x%"PRIx64"\n", c->cr3); return -EINVAL;xen/arch/x86/hvm/svm/svm.c +2
-1@@ -257,6 +258,8 @@ static int svm_vmcb_restore(struct vcpuput_page (pagetable_get_page (v->arch.guest_table)); v->arch.guest_table =pagetable_from_pfn (mfn); +if ( c->cr0 &X86_CR0_PG ) +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT ); } v->arch.hvm_vcpu.guest_cr[0 ] = c->cr0 |X86_CR0_ET ;@@ -1144,7 +1147,6 @@ static void svm_do_nested_pgfault(structunsigned long gfn = gpa >>PAGE_SHIFT ;mfn_t mfn;p2m_type_t p2mt; -p2m_access_t p2ma;struct p2m_domain *p2m =NULL ; ret =hvm_hap_nested_page_fault (gpa,0 , ~0ul ,0 ,0 ,0 ,0 );xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1168
Quoted 7 lines
xen/arch/x86/hvm/svm/svm.c +2
-1@@ -1161,7 +1163,8 @@ static void svm_do_nested_pgfault(struct p2m =p2m_get_p2m (v); _d.gpa = gpa; _d.qualification =0 ; - _d.mfn =mfn_x (gfn_to_mfn_type_p2m (p2m, gfn, &_d.p2mt, &p2ma, p2m_query, NULL)); + mfn =get_gfn_query_unlocked (p2m->domain, gfn, &_d.p2mt); + _d.mfn =mfn_x (mfn);Ah - this is not quite the same thing. This lookup uses a specific p2m table (possibly a nested-p2m table reflecting the fact the guest is running in nested SVM mode) so it can't be replaced by a call that just takes the domain pointer (and will internally use the domain's master p2m table).
The naming of '
p2m_get_p2m()' is particularly unhelpful here, I realise. It fetches the running p2m, i.e. the one that hardware sees as an NPT table; almost everywhere else usesp2m_get_hostp2m(), which fetches the master p2m. But in general when you see the gfn_to_mfn_*_p2m() functions, that take an explicit p2m pointer, you need to be careful.xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1187
Quoted 9 lines
xen/arch/x86/hvm/svm/svm.c +1
-1__trace_var (TRC_HVM_NPF ,0 ,sizeof (_d), &_d); }@@ -1181,7 +1184,7 @@ static void svm_do_nested_pgfault(structif ( p2m ==NULL ) p2m =p2m_get_p2m (v);/* Everything else is an error. */ - mfn =gfn_to_mfn_type_p2m (p2m, gfn, &p2mt, &p2ma, p2m_guest,NULL ); + mfn =get_gfn_guest_unlocked (p2m->domain, gfn, &p2mt);Likewise here.
Quoted 11 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/guest_walk.cxen/arch/x86/mm/guest_walk.c +2
-0@@ -86,30 +86,33 @@ static uint32_t set_ad_bits(void *guest_return 0 ; } +/* If the map is non-NULL, we leave this function having +* called get_gfn, you need to call put_gfn. */ static inline void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn,xen/arch/x86/mm/guest_walk.c static uint32_t set_ad_bits(void *guest_ · line 99
Quoted 10 lines
xen/arch/x86/mm/guest_walk.c +1
-3mfn_t *mfn, p2m_type_t *p2mt, uint32_t *rc) { -p2m_access_t a; -/* Translate the gfn, unsharing if shared */ - *mfn =gfn_to_mfn_type_p2m (p2m,gfn_x (gfn), p2mt, &a, p2m_unshare, NULL); + *mfn =get_gfn_unshare (p2m->domain,gfn_x (gfn), p2mt);Here's another case where we need to handle the nested-hap path; the p2m pointer here must be propagated into the lookup function.
Quoted 21 lines
xen/arch/x86/mm/guest_walk.c +3
-0if (p2m_is_paging (*p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain,gfn_x (gfn)); +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_PAGED;return NULL ; }if (p2m_is_shared (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_SHARED;return NULL ; }if ( !p2m_is_ram (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc |= _PAGE_PRESENT;return NULL ;}xen/arch/x86/mm/guest_walk.c static inline void *map_domain_gfn(struc · line 129
Quoted 7 lines
xen/arch/x86/mm/guest_walk.c +3
-0@@ -120,6 +123,9 @@ static inline void *map_domain_gfn(struc/* Walk the guest pagetables, after the manner of a hardware walker. */ +/* Because the walk is essentially random, it can cause a deadlock +* warning in the p2m locking code. Highly unlikely this is an actual +* deadlock, because who would walk page table in the opposite order? */ Linear pagetables make this sort of thing more likely, especially if they're used by one process to update another process's mappings.
If this is a problem, maybe we'll need to avoid the deadlock either by allowing multiple lock-holders in the p2m or by rearranging the a/d writeback soit does another p2m lookup -- I'd rather not do that, though, since even on 64-bit it will add a lot of memory accesses.
I'm happy to take a version of this big switchover patch that doesn't solve this problem, though. It can be sorted out in a separate patch.
xen/arch/x86/mm/hap/nested_hap.c nestedhap_walk_L0_p2m(struct p2m_domain · line 153
Quoted 12 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/hap/nested_hap.cxen/arch/x86/mm/hap/nested_hap.c +1
-0@@ -146,22 +146,29 @@ nestedhap_walk_L0_p2m(struct p2m_domainmfn_t mfn;p2m_type_t p2mt;p2m_access_t p2ma; +int rc;/* walk L0 P2M table */ mfn =gfn_to_mfn_type_p2m (p2m, L1_gpa >>PAGE_SHIFT , &p2mt, &p2ma, p2m_query, page_order);Again, are we not changing this function's name?
Cheers,
Tim.
Re: [PATCH 5 of 5] Modify naming of queries into the p2m
In reply to Tim DeeganTim, see below re nested-SVM code:
Quoted 65 lines
Hi,
This mostly looks good; I have a few comments on the detail below. If those are addressed I hope I can apply the next version.
Any other maintainers want to object to this renaming?
Quoted 9 lines · Andres Lagar-Cavilla
xen/arch/x86/hvm/hvm.c +1
-0@@ -1182,6 +1195,7 @@ int hvm_hap_nested_page_fault(unsigned lmfn_t mfn;struct vcpu *v = current;struct p2m_domain *p2m; +int rc;/* On Nested Virtualization, walk the guest page table. * If this succeeds, all is fine. xen/arch/x86/hvm/hvm.c int hvm_hap_nested_page_fault(unsigned l · line 1274
Quoted 8 lines
xen/arch/x86/hvm/hvm.c +2
-2@@ -1255,8 +1269,8 @@ int hvm_hap_nested_page_fault(unsigned lif ( violation ) {p2m_mem_access_check (gpa, gla_valid, gla, access_r, access_w, access_x); - -return 1 ; + rc =1 ; + goto out_put_gfn;Shouldn't this patch be changing the call to
gfn_to_mfn_type_p2m()just above here to a get_gfn_*() call too?Quoted 20 lines
xen/arch/x86/hvm/hvm.c +2
-1-void hvm_unmap_guest_frame (void *p) +void hvm_unmap_guest_frame (void *p,unsigned long addr,int is_va) { +/* We enter this function with a map obtained in
__hvm_map_guest_frame.xen/arch/x86/hvm/hvm.c +14
-0+* This map performed a p2m query that locked the gfn entry and got +* a ref on the mfn. Must undo */ if ( p ) + { +unsigned long gfn =INVALID_GFN ; + +if ( is_va ) + { +if ( addr ) + { +uint32_t pfec = PFEC_page_present; + gfn =paging_gva_to_gfn (current, addr, &pfec); + }else { + gfn =INVALID_GFN ; + }xen/arch/x86/hvm/hvm.c void *hvm_map_guest_frame_ro(unsigned lo · line 1862
Quoted 10 lines
xen/arch/x86/hvm/hvm.c +8
-0+ }else { + gfn = addr; + } + +if ( gfn !=INVALID_GFN ) +put_gfn (current->domain, gfn); +unmap_domain_page (p); + } }This is a pretty wierd-looking function now - I think it would be better just to make all callers have to remember the GFN. In fact, since a guest VCPU can change its pagetables at any time, it's not safe to use
paging_gva_to_gfn()to regenerate the GFN from a VA.
Quoted 3 lines
Also, IIRC the nested-SVM code keeps the
hvm_mapmapping around for quite a long time so in fact this unmap-and-put-gfn may not be the right interface. Maybe the caller shouldput_gfn()explicitly.
I just looked into nestedsvm_vmcb_map. It would seem that the map it stores in nv_vvmcx survives beyond the vmexit. So, if we don't put_gfn we would be entering the scheduler in an atomic context.
The shorthand solution is to put_gfn, but that would render the map potentially unsafe on re-entry. I don't think so, so I'll push forward with that.
Is it possible to teardown and re-establish the maps on each hypervisor entry/exit? That would seem like the ultimate solution.
Andres
Quoted 155 lines
Quoted 37 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/hvm/svm/svm.cxen/arch/x86/hvm/svm/svm.c +2
-1@@ -244,9 +244,10 @@ static int svm_vmcb_restore(struct vcpu {if ( c->cr0 &X86_CR0_PG ) { - mfn =mfn_x (gfn_to_mfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt)); + mfn =mfn_x (get_gfn (v->domain, c->cr3 >>PAGE_SHIFT , &p2mt));if ( !p2m_is_ram (p2mt) || !get_page (mfn_to_page (mfn), v->domain) ) { +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT );gdprintk(XENLOG_ERR, "Invalid CR3 value=0x%"PRIx64"\n", c->cr3); return -EINVAL;xen/arch/x86/hvm/svm/svm.c +2
-1@@ -257,6 +258,8 @@ static int svm_vmcb_restore(struct vcpuput_page (pagetable_get_page (v->arch.guest_table)); v->arch.guest_table =pagetable_from_pfn (mfn); +if ( c->cr0 &X86_CR0_PG ) +put_gfn (v->domain, c->cr3 >>PAGE_SHIFT ); } v->arch.hvm_vcpu.guest_cr[0 ] = c->cr0 |X86_CR0_ET ;@@ -1144,7 +1147,6 @@ static void svm_do_nested_pgfault(structunsigned long gfn = gpa >>PAGE_SHIFT ;mfn_t mfn;p2m_type_t p2mt; -p2m_access_t p2ma;struct p2m_domain *p2m =NULL ; ret =hvm_hap_nested_page_fault (gpa,0 , ~0ul ,0 ,0 ,0 ,0 );xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1168
Quoted 7 lines
xen/arch/x86/hvm/svm/svm.c +2
-1@@ -1161,7 +1163,8 @@ static void svm_do_nested_pgfault(struct p2m =p2m_get_p2m (v); _d.gpa = gpa; _d.qualification =0 ; - _d.mfn =mfn_x (gfn_to_mfn_type_p2m (p2m, gfn, &_d.p2mt, &p2ma, p2m_query, NULL)); + mfn =get_gfn_query_unlocked (p2m->domain, gfn, &_d.p2mt); + _d.mfn =mfn_x (mfn);Ah - this is not quite the same thing. This lookup uses a specific p2m table (possibly a nested-p2m table reflecting the fact the guest is running in nested SVM mode) so it can't be replaced by a call that just takes the domain pointer (and will internally use the domain's master p2m table).
The naming of '
p2m_get_p2m()' is particularly unhelpful here, I realise. It fetches the running p2m, i.e. the one that hardware sees as an NPT table; almost everywhere else usesp2m_get_hostp2m(), which fetches the master p2m. But in general when you see the gfn_to_mfn_*_p2m() functions, that take an explicit p2m pointer, you need to be careful.xen/arch/x86/hvm/svm/svm.c static void svm_do_nested_pgfault(struct · line 1187
Quoted 9 lines
xen/arch/x86/hvm/svm/svm.c +1
-1__trace_var (TRC_HVM_NPF ,0 ,sizeof (_d), &_d); }@@ -1181,7 +1184,7 @@ static void svm_do_nested_pgfault(structif ( p2m ==NULL ) p2m =p2m_get_p2m (v);/* Everything else is an error. */ - mfn =gfn_to_mfn_type_p2m (p2m, gfn, &p2mt, &p2ma, p2m_guest,NULL ); + mfn =get_gfn_guest_unlocked (p2m->domain, gfn, &p2mt);Likewise here.
Quoted 11 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/guest_walk.cxen/arch/x86/mm/guest_walk.c +2
-0@@ -86,30 +86,33 @@ static uint32_t set_ad_bits(void *guest_return 0 ; } +/* If the map is non-NULL, we leave this function having +* called get_gfn, you need to call put_gfn. */ static inline void *map_domain_gfn(struct p2m_domain *p2m, gfn_t gfn,xen/arch/x86/mm/guest_walk.c static uint32_t set_ad_bits(void *guest_ · line 99
Quoted 10 lines
xen/arch/x86/mm/guest_walk.c +1
-3mfn_t *mfn, p2m_type_t *p2mt, uint32_t *rc) { -p2m_access_t a; -/* Translate the gfn, unsharing if shared */ - *mfn =gfn_to_mfn_type_p2m (p2m,gfn_x (gfn), p2mt, &a, p2m_unshare, NULL); + *mfn =get_gfn_unshare (p2m->domain,gfn_x (gfn), p2mt);Here's another case where we need to handle the nested-hap path; the p2m pointer here must be propagated into the lookup function.
Quoted 21 lines
xen/arch/x86/mm/guest_walk.c +3
-0if (p2m_is_paging (*p2mt) ) {ASSERT (!p2m_is_nestedp2m (p2m));p2m_mem_paging_populate (p2m->domain,gfn_x (gfn)); +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_PAGED;return NULL ; }if (p2m_is_shared (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc = _PAGE_SHARED;return NULL ; }if ( !p2m_is_ram (*p2mt) ) { +put_gfn (p2m->domain,gfn_x (gfn)); *rc |= _PAGE_PRESENT;return NULL ;}xen/arch/x86/mm/guest_walk.c static inline void *map_domain_gfn(struc · line 129
Quoted 7 lines
xen/arch/x86/mm/guest_walk.c +3
-0@@ -120,6 +123,9 @@ static inline void *map_domain_gfn(struc/* Walk the guest pagetables, after the manner of a hardware walker. */ +/* Because the walk is essentially random, it can cause a deadlock +* warning in the p2m locking code. Highly unlikely this is an actual +* deadlock, because who would walk page table in the opposite order? */ Linear pagetables make this sort of thing more likely, especially if they're used by one process to update another process's mappings.
If this is a problem, maybe we'll need to avoid the deadlock either by allowing multiple lock-holders in the p2m or by rearranging the a/d writeback soit does another p2m lookup -- I'd rather not do that, though, since even on 64-bit it will add a lot of memory accesses.
I'm happy to take a version of this big switchover patch that doesn't solve this problem, though. It can be sorted out in a separate patch.
xen/arch/x86/mm/hap/nested_hap.c nestedhap_walk_L0_p2m(struct p2m_domain · line 153
Quoted 12 lines
diff -r
f07d4ebe5248-re9fd07479f68xen/arch/x86/mm/hap/nested_hap.cxen/arch/x86/mm/hap/nested_hap.c +1
-0@@ -146,22 +146,29 @@ nestedhap_walk_L0_p2m(struct p2m_domainmfn_t mfn;p2m_type_t p2mt;p2m_access_t p2ma; +int rc;/* walk L0 P2M table */ mfn =gfn_to_mfn_type_p2m (p2m, L1_gpa >>PAGE_SHIFT , &p2mt, &p2ma, p2m_query, page_order);Again, are we not changing this function's name?
Cheers,
Tim.
Quoted 15 lines · Andres Lagar-Cavilla
This patch series lays the groundwork for improving the synchronization of primitives accessing the p2m.
This is a partial repost of the patches emailed previously as a RFC. These patches are now intended for committing to the tree.
We change the API for accessing the p2m to a family of functions get_gfn/put_gfn. The name intends to reflect the fact that even lookups are meant to obtain exclusive access to a p2m entry, and that said access should be relinquished (
put_gfn) when done.The patches, however, alter little functionality. The API name change does not involve yet additional locking or ref-counting. They will, however, throw a "barrier" that will force any new commits to conform to the new API.
Quoted 5 lines
Patches are based off 24066:54a5e994a241. Should the new XENMEM calls be accepted before this, the series needs to be updated to also change the API there.
Signed-off-by Andres Lagar-Cavilla <andres@lagarcavilla.
Patches 0-4 applied, thanks. I've reviewed patch 5 separately.
Cheers,
Tim.