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. Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> 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(-)
Andres Lagar-Cavilla
2012-Feb-16 03:42 UTC
[PATCH 1 of 4] 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(-)
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 658530e2f380 -r a70a87d7bf84 xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -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;
}
diff -r 658530e2f380 -r a70a87d7bf84 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -735,6 +735,7 @@ set_shared_p2m_entry(struct domain *d, u
p2m_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));
Andres Lagar-Cavilla
2012-Feb-16 03:42 UTC
[PATCH 2 of 4] x86/mm: Fix more ballooning+paging and ballooning+sharing bugs
xen/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
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r a70a87d7bf84 -r b03a10be1428 xen/arch/x86/mm/p2m.c
--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ -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);
}
/**
diff -r a70a87d7bf84 -r b03a10be1428 xen/common/memory.c
--- a/xen/common/memory.c
+++ b/xen/common/memory.c
@@ -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);
Andres Lagar-Cavilla
2012-Feb-16 03:42 UTC
[PATCH 3 of 4] x86/mm: Check sharing/paging/access have been enabled before processing a memop
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 +-
4 files changed, 11 insertions(+), 3 deletions(-)
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r b03a10be1428 -r 7a1d415a71d0 xen/arch/x86/mm/mem_access.c
--- a/xen/arch/x86/mm/mem_access.c
+++ b/xen/arch/x86/mm/mem_access.c
@@ -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:
diff -r b03a10be1428 -r 7a1d415a71d0 xen/arch/x86/mm/mem_event.c
--- a/xen/arch/x86/mm/mem_event.c
+++ b/xen/arch/x86/mm/mem_event.c
@@ -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)
diff -r b03a10be1428 -r 7a1d415a71d0 xen/arch/x86/mm/mem_paging.c
--- a/xen/arch/x86/mm/mem_paging.c
+++ b/xen/arch/x86/mm/mem_paging.c
@@ -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:
diff -r b03a10be1428 -r 7a1d415a71d0 xen/arch/x86/mm/mem_sharing.c
--- a/xen/arch/x86/mm/mem_sharing.c
+++ b/xen/arch/x86/mm/mem_sharing.c
@@ -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)
xen/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.
Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>
diff -r 7a1d415a71d0 -r 62b1fe67b8d1 xen/arch/x86/hvm/vmx/vmx.c
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1010,12 +1010,22 @@ static void vmx_load_pdptrs(struct vcpu
if ( (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 vcpu
vmx_vmcs_exit(v);
unmap_domain_page(p);
- put_gfn(v->domain, cr3 >> PAGE_SHIFT);
+ put_page(mfn_to_page(mfn));
return;
crash:
diff -r 7a1d415a71d0 -r 62b1fe67b8d1 xen/arch/x86/mm/hap/hap.c
--- a/xen/arch/x86/mm/hap/hap.c
+++ b/xen/arch/x86/mm/hap/hap.c
@@ -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
At 22:42 -0500 on 15 Feb (1329345744), Andres Lagar-Cavilla wrote:> 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.> - 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.This needed a minor adjustment to a printk format for 32-bit builds.> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org>All applied, thanks. Cheers, Tim.
> At 22:42 -0500 on 15 Feb (1329345744), Andres Lagar-Cavilla wrote: >> 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> >> - 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. > > This needed a minor adjustment to a printk format for 32-bit builds. > >> Signed-off-by: Andres Lagar-Cavilla <andres@lagarcavilla.org> > > All applied, thanks. > > Cheers, > > Tim. >