x86/mm: Four fixes
PATCH 1/1 x86/mm: Four fixes
In this series we post four patches with bugfixes to p2m, hap, paging and sharing code:
- Make a few asserts on shared pages type and counts more accurate (this time done right, hopefully!)
- Bugfix interactions between the balloon and the paging and sharing subsystems. Posted a week ago, probably slipped through the cracks.
- Added a missing sanity check for sharing/paging/access memops.
- Fix vmx_load_pdptrs to crash the guest instead of the host in the presence of paged out cr3's.
xen/arch/x86/mm/mem_sharing.c | 3 ++- xen/arch/x86/mm/p2m.c | 7 +++++-- xen/arch/x86/mm/p2m.c | 7 +++++-- xen/common/memory.c | 17 ++++++++++++++++- xen/arch/x86/mm/mem_access.c | 3 +++ xen/arch/x86/mm/mem_event.c | 6 ++++-- xen/arch/x86/mm/mem_paging.c | 3 +++ xen/arch/x86/mm/mem_sharing.c | 2 +- xen/arch/x86/hvm/vmx/vmx.c | 16 +++++++++++++--- xen/arch/x86/mm/hap/hap.c | 2 +- 10 files changed, 53 insertions(+), 13 deletions(-)
PATCH 1/1 x86/mm: Make asserts on types and counts of shared pages more accurate
xen/arch/x86/mm/mem_sharing.c | 3 ++- xen/arch/x86/mm/p2m.c | 7 +++++-- 2 files changed, 7 insertions(+), 3 deletions(-)
xen/arch/x86/mm/mem_sharing.c +2 -1
@@ -201,7 +201,8 @@ static struct page_info* mem_sharing_loo/* Count has to be at least two, because we're called * with the mfn locked (1) and this is supposed to be * a shared page (1). */ -ASSERT ((page->u.inuse.type_info & PGT_count_mask) >=2 ); +unsigned long type =read_atomic (&page->u.inuse.type_info); +ASSERT ((type & PGT_shared_page) && ((type & PGT_count_mask) >=2 ));ASSERT (get_gpfn_from_mfn (mfn) ==SHARED_M2P_ENTRY );return page; }
xen/arch/x86/mm/p2m.c +5 -2
@@ -735,6 +735,7 @@ set_shared_p2m_entry(struct domain *d, up2m_access_t a;p2m_type_t ot;mfn_t omfn; +unsigned long pg_type;if ( !paging_mode_translate (p2m->domain) )return 0 ;
@@ -745,8 +746,10 @@ set_shared_p2m_entry(struct domain *d, u* sharable first */ ASSERT (p2m_is_shared (ot));ASSERT (mfn_valid (omfn)); -if ( ((mfn_to_page (omfn)->u.inuse.type_info & PGT_type_mask) - != PGT_shared_page) ) +/* Set the m2p entry to invalid only if there are no further type +* refs to this page as shared */ + pg_type =read_atomic (&(mfn_to_page (omfn)->u.inuse.type_info)); +if ( !((pg_type & PGT_shared_page) && (pg_type & PGT_count_mask)) )set_gpfn_from_mfn (mfn_x (omfn),INVALID_M2P_ENTRY );P2M_DEBUG ("set shared %lx %lx\n" , gfn,mfn_x (mfn));
PATCH 1/1 x86/mm: Fix more ballooning+paging and ballooning+sharing bugs
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/p2m.c | 7 +++++-- xen/common/memory.c | 17 ++++++++++++++++- 2 files changed, 21 insertions(+), 3 deletions(-)
If the guest balloons away a page that has been nominated for paging but not yet paged out, we fix:
- Send EVICT_FAIL flag in the event to the pager
- Do not leak the underlying page
If the page was shared, we were not:
- properly refreshing the mfn to balloon after the unshare.
- unlocking the p2m on the error exit case
xen/arch/x86/mm/p2m.c +5 -2
@@ -928,11 +928,14 @@ void p2m_mem_paging_drop_page(struct dom req.gfn = gfn; req.flags =MEM_EVENT_FLAG_DROP_PAGE ; -mem_event_put_request (d, &d->mem_event->paging, &req); -/* Update stats unless the page hasn't yet been evicted */ if ( p2mt != p2m_ram_paging_out )atomic_dec (&d->paged_pages); +else +/* Evict will fail now, tag this request for pager */ + req.flags |=MEM_EVENT_FLAG_EVICT_FAIL ; + +mem_event_put_request (d, &d->mem_event->paging, &req); }/**
xen/common/memory.c +16 -1
@@ -167,6 +167,15 @@ int guest_remove_page(struct domain *d, {guest_physmap_remove_page (d, gmfn, mfn,0 );put_gfn (d, gmfn); +/* If the page hasn't yet been paged out, there is an +* actual page that needs to be released. */ +if ( p2mt == p2m_ram_paging_out ) + { +ASSERT (mfn_valid (mfn)); + page =mfn_to_page (mfn); +if (test_and_clear_bit (_PGC_allocated, &page->count_info) ) +put_page (page); + }p2m_mem_paging_drop_page (d, gmfn, p2mt);return 1 ; }
@@ -181,7 +190,6 @@ int guest_remove_page(struct domain *d,return 0 ; } - page =mfn_to_page (mfn);#ifdef CONFIG_X86_64 if (p2m_is_shared (p2mt) ) {
@@ -190,10 +198,17 @@ int guest_remove_page(struct domain *d,* need to trigger proper cleanup. Once done, this is * like any other page. */ if (mem_sharing_unshare_page (d, gmfn,0 ) ) + { +put_gfn (d, gmfn);return 0 ; + } +/* Maybe the mfn changed */ + mfn =mfn_x (get_gfn_query_unlocked (d, gmfn, &p2mt)); + ASSERT(!p2m_is_shared(p2mt)); }#endif /* CONFIG_X86_64 */ + page =mfn_to_page (mfn);if (unlikely (!get_page (page, d)) ) {put_gfn (d, gmfn);
PATCH 1/1 x86/mm: Check sharing/paging/access have been enabled before processing a memop
In reply to Andres Lagar-Cavillaxen/arch/x86/mm/mem_access.c | 3 +++ xen/arch/x86/mm/mem_event.c | 6 ++++-- xen/arch/x86/mm/mem_paging.c | 3 +++ xen/arch/x86/mm/mem_sharing.c | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-)
xen/arch/x86/mm/mem_access.c +3 -0
@@ -29,6 +29,9 @@ int mem_access_memop(struct domain *d, x {int rc; +if (unlikely (!d->mem_event->access.ring_page) ) +return -ENODEV ; +switch ( meo->op ) {case XENMEM_access_op_resume:
xen/arch/x86/mm/mem_event.c +4 -2
@@ -452,13 +452,15 @@ int mem_event_claim_slot(struct domain */* Registered with Xen-bound event channel for incoming notifications. */ static void mem_paging_notification (struct vcpu *v,unsigned int port) { -p2m_mem_paging_resume (v->domain); +if (likely (v->domain->mem_event->paging.ring_page !=NULL ) ) +p2m_mem_paging_resume (v->domain); }/* Registered with Xen-bound event channel for incoming notifications. */ static void mem_access_notification (struct vcpu *v,unsigned int port) { -p2m_mem_access_resume (v->domain); +if (likely (v->domain->mem_event->access.ring_page !=NULL ) ) +p2m_mem_access_resume (v->domain); }struct domain *get_mem_event_op_target (uint32_t domain,int *rc)
xen/arch/x86/mm/mem_paging.c +3 -0
@@ -27,6 +27,9 @@int mem_paging_memop (struct domain *d,xen_mem_event_op_t *mec) { +if (unlikely (!d->mem_event->paging.ring_page) ) +return -ENODEV ; +switch ( mec->op ) {case XENMEM_paging_op_nominate:
xen/arch/x86/mm/mem_sharing.c +1 -1
@@ -1021,7 +1021,7 @@ int mem_sharing_memop(struct domain *d,int rc =0 ;/* Only HAP is supported */ -if ( !hap_enabled (d) ) +if ( !hap_enabled (d) || !d->arch.hvm_domain.mem_sharing_enabled )return -ENODEV ; switch(mec->op)
PATCH 1/1 x86/mm: Fix two PAE+paging bugs
In reply to Andres Lagar-Cavillaxen/arch/x86/hvm/vmx/vmx.c | 16 +++++++++++++--- xen/arch/x86/mm/hap/hap.c | 2 +- 2 files changed, 14 insertions(+), 4 deletions(-)
In hap_paging_update_modes, we were getting the gpa of the cr3, rather than the gfn.
Vmx_load_pdptrs was crashing the host if the cr3 is paged out. Now it will only crash the guest.
xen/arch/x86/hvm/vmx/vmx.c +13 -3
@@ -1010,12 +1010,22 @@ static void vmx_load_pdptrs(struct vcpuif ( (cr3 &0x1fUL ) && !hvm_pcid_enabled (v) ) goto crash; - mfn =mfn_x (get_gfn (v->domain, cr3 >>PAGE_SHIFT , &p2mt)); -if ( !p2m_is_ram (p2mt) ) + mfn =mfn_x (get_gfn_unshare (v->domain, cr3 >>PAGE_SHIFT , &p2mt)); +if ( !p2m_is_ram (p2mt) || !mfn_valid (mfn) || +/* If we didn't succeed in unsharing, get_page will fail +* (page still belongs to dom_cow) */ + !get_page (mfn_to_page (mfn), v->domain) ) { +/* Ideally you don't want to crash but rather go into a wait +* queue, but this is the wrong place. We're holding at least +* the paging lock */ +gdprintk (XENLOG_ERR , +"Bad cr3 on load pdptrs gfn %" PRIx64" mfn %" PRIx64 +" type %d\n" ,cr3 >>PAGE_SHIFT , mfn, (int )p2mt);put_gfn (v->domain, cr3 >>PAGE_SHIFT ); goto crash; } +put_gfn (v->domain, cr3 >>PAGE_SHIFT ); p =map_domain_page (mfn);
@@ -1043,7 +1053,7 @@ static void vmx_load_pdptrs(struct vcpuvmx_vmcs_exit (v);unmap_domain_page (p); -put_gfn (v->domain, cr3 >>PAGE_SHIFT ); +put_page (mfn_to_page (mfn));return ; crash:
xen/arch/x86/mm/hap/hap.c +1 -1
@@ -786,7 +786,7 @@ hap_paging_get_mode(struct vcpu *v)static void hap_update_paging_modes (struct vcpu *v) {struct domain *d = v->domain; -unsigned long cr3_gfn = v->arch.hvm_vcpu.guest_cr[3 ]; +unsigned long cr3_gfn = v->arch.hvm_vcpu.guest_cr[3 ] >>PAGE_SHIFT ;p2m_type_t t;/* We hold onto the cr3 as it may be modified later, and
Quoted 5 lines · Andres Lagar-Cavilla
In this series we post four patches with bugfixes to p2m, hap, paging and sharing code:
- Make a few asserts on shared pages type and counts more accurate (this time done right, hopefully!)
Nope, sorry! :) I fixed the tests up and applied it; please check that it still does what you wanted.
Quoted 5 lines
- Bugfix interactions between the balloon and the paging and sharing subsystems. Posted a week ago, probably slipped through the cracks.
- Added a missing sanity check for sharing/paging/access memops.
- Fixvmx_load_pdptrsto crash the guest instead of the host in the presence of paged out cr3's.
This needed a minor adjustment to a printk format for 32-bit builds.
Quoted 1 line
Signed-off-by Andres Lagar-Cavilla <[email protected]>
All applied, thanks.
Cheers,
Tim.
Quoted 10 lines
Quoted 6 lines · Andres Lagar-Cavilla
In this series we post four patches with bugfixes to p2m, hap, paging and
sharing code:- Make a few asserts on shared pages type and counts more accurate (this time done right, hopefully!)
Nope, sorry! :) I fixed the tests up and applied it; please check that it still does what you wanted.
Check. It wasn't that bad to begin with :)
Thanks
Andres
Quoted 17 lines
Quoted 6 lines
- Bugfix interactions between the balloon and the paging and sharing subsystems. Posted a week ago, probably slipped through the cracks.
- Added a missing sanity check for sharing/paging/access memops.
- Fixvmx_load_pdptrsto crash the guest instead of the host in the presence of paged out cr3's.This needed a minor adjustment to a printk format for 32-bit builds.
Quoted 1 line
Signed-off-by Andres Lagar-Cavilla <[email protected]>All applied, thanks.
Cheers,
Tim.