[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 5 of 6] Rework stale p2m auditing
On Mon, Nov 14, 2011 at 9:48 PM, Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> wrote: > xen/arch/x86/domctl.c | 24 +++++++ > xen/arch/x86/mm/p2m-ept.c | 1 + > xen/arch/x86/mm/p2m-pod.c | 5 - > xen/arch/x86/mm/p2m-pt.c | 137 > +++++++------------------------------------ > xen/arch/x86/mm/p2m.c | 124 ++++++++++++++++++++++++++++++++++++--- > xen/include/asm-x86/p2m.h | 11 ++- > xen/include/public/domctl.h | 12 +++ > 7 files changed, 180 insertions(+), 134 deletions(-) > > > The p2m audit code doesn't even compile, let alone work. It also > partially supports ept. Make it: > > - compile > - lay groundwork for eventual ept support This stuff looks good (haven't reviewed it in detail), but... > - move out of the way of all calls and turn it into a domctl. It's > obviously not being used by anybody presently. > - enable it via said domctl ..the idea with having audit_p2m() sprinkled around the code was to help narrow down what bit of code was screwing up the p2m table. I'm not sure switching it to a domctl will really help debugging in the future. How were you envisioning this would be used? > > Signed-off-by: Andres Lagar-Cavilla <andres@xxxxxxxxxxxxxxxx> > > diff -r f11528df1df3 -r 764e0872dd4f xen/arch/x86/domctl.c > --- a/xen/arch/x86/domctl.c > +++ b/xen/arch/x86/domctl.c > @@ -1449,6 +1449,30 @@ long arch_do_domctl( > break; > #endif /* __x86_64__ */ > > + case XEN_DOMCTL_audit_p2m: > + { > + struct domain *d; > + > + ret = -ESRCH; > + d = rcu_lock_domain_by_id(domctl->domain); > + if ( d != NULL ) > + { > +#if P2M_AUDIT > + audit_p2m(d, > + &domctl->u.audit_p2m.orphans_debug, > + &domctl->u.audit_p2m.orphans_invalid, > + &domctl->u.audit_p2m.m2p_bad, > + &domctl->u.audit_p2m.p2m_bad); > + ret = (copy_to_guest(u_domctl, domctl, 1)) ? > + -EFAULT : 0; > +#else > + ret = -ENOSYS; > +#endif /* P2M_AUDIT */ > + rcu_unlock_domain(d); > + } > + } > + break; > + > case XEN_DOMCTL_set_access_required: > { > struct domain *d; > diff -r f11528df1df3 -r 764e0872dd4f xen/arch/x86/mm/p2m-ept.c > --- a/xen/arch/x86/mm/p2m-ept.c > +++ b/xen/arch/x86/mm/p2m-ept.c > @@ -817,6 +817,7 @@ void ept_p2m_init(struct p2m_domain *p2m > p2m->set_entry = ept_set_entry; > p2m->get_entry = ept_get_entry; > p2m->change_entry_type_global = ept_change_entry_type_global; > + p2m->audit_p2m = NULL; > } > > static void ept_dump_p2m_table(unsigned char key) > diff -r f11528df1df3 -r 764e0872dd4f xen/arch/x86/mm/p2m-pod.c > --- a/xen/arch/x86/mm/p2m-pod.c > +++ b/xen/arch/x86/mm/p2m-pod.c > @@ -522,7 +522,6 @@ p2m_pod_decrease_reservation(struct doma > steal_for_cache = ( p2m->pod.entry_count > p2m->pod.count ); > > p2m_lock(p2m); > - audit_p2m(p2m, 1); > > if ( unlikely(d->is_dying) ) > goto out_unlock; > @@ -616,7 +615,6 @@ out_entry_check: > } > > out_unlock: > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > > out: > @@ -986,7 +984,6 @@ p2m_pod_demand_populate(struct p2m_domai > */ > set_p2m_entry(p2m, gfn_aligned, _mfn(0), PAGE_ORDER_2M, > p2m_populate_on_demand, p2m->default_access); > - audit_p2m(p2m, 1); > return 0; > } > > @@ -1108,7 +1105,6 @@ guest_physmap_mark_populate_on_demand(st > return rc; > > p2m_lock(p2m); > - audit_p2m(p2m, 1); > > P2M_DEBUG("mark pod gfn=%#lx\n", gfn); > > @@ -1142,7 +1138,6 @@ guest_physmap_mark_populate_on_demand(st > BUG_ON(p2m->pod.entry_count < 0); > } > > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > > out: > diff -r f11528df1df3 -r 764e0872dd4f xen/arch/x86/mm/p2m-pt.c > --- a/xen/arch/x86/mm/p2m-pt.c > +++ b/xen/arch/x86/mm/p2m-pt.c > @@ -483,7 +483,6 @@ static int p2m_pod_check_and_populate(st > /* This is called from the p2m lookups, which can happen with or > * without the lock hed. */ > p2m_lock_recursive(p2m); > - audit_p2m(p2m, 1); > > /* Check to make sure this is still PoD */ > if ( p2m_flags_to_type(l1e_get_flags(*p2m_entry)) != > p2m_populate_on_demand ) > @@ -494,7 +493,6 @@ static int p2m_pod_check_and_populate(st > > r = p2m_pod_demand_populate(p2m, gfn, order, q); > > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > > return r; > @@ -975,118 +973,23 @@ static void p2m_change_type_global(struc > > } > > -/* Set up the p2m function pointers for pagetable format */ > -void p2m_pt_init(struct p2m_domain *p2m) > +#if P2M_AUDIT > +long p2m_pt_audit_p2m(struct p2m_domain *p2m) > { > - p2m->set_entry = p2m_set_entry; > - p2m->get_entry = p2m_gfn_to_mfn; > - p2m->change_entry_type_global = p2m_change_type_global; > - p2m->write_p2m_entry = paging_write_p2m_entry; > -} > - > - > -#if P2M_AUDIT > -/* strict_m2p == 0 allows m2p mappings that don'#t match the p2m. > - * It's intended for add_to_physmap, when the domain has just been allocated > - * new mfns that might have stale m2p entries from previous owners */ > -void audit_p2m(struct p2m_domain *p2m, int strict_m2p) > -{ > - struct page_info *page; > - struct domain *od; > - unsigned long mfn, gfn, m2pfn, lp2mfn = 0; > int entry_count = 0; > - mfn_t p2mfn; > - unsigned long orphans_d = 0, orphans_i = 0, mpbad = 0, pmbad = 0; > + unsigned long pmbad = 0; > + unsigned long mfn, gfn, m2pfn; > int test_linear; > - p2m_type_t type; > struct domain *d = p2m->domain; > > - if ( !paging_mode_translate(d) ) > - return; > - > - //P2M_PRINTK("p2m audit starts\n"); > + ASSERT(p2m_locked_by_me(p2m)); > > test_linear = ( (d == current->domain) > && !pagetable_is_null(current->arch.monitor_table) ); > if ( test_linear ) > flush_tlb_local(); > > - spin_lock(&d->page_alloc_lock); > - > - /* Audit part one: walk the domain's page allocation list, checking > - * the m2p entries. */ > - page_list_for_each ( page, &d->page_list ) > - { > - mfn = mfn_x(page_to_mfn(page)); > - > - // P2M_PRINTK("auditing guest page, mfn=%#lx\n", mfn); > - > - od = page_get_owner(page); > - > - if ( od != d ) > - { > - P2M_PRINTK("wrong owner %#lx -> %p(%u) != %p(%u)\n", > - mfn, od, (od?od->domain_id:-1), d, d->domain_id); > - continue; > - } > - > - gfn = get_gpfn_from_mfn(mfn); > - if ( gfn == INVALID_M2P_ENTRY ) > - { > - orphans_i++; > - //P2M_PRINTK("orphaned guest page: mfn=%#lx has invalid gfn\n", > - // mfn); > - continue; > - } > - > - if ( gfn == 0x55555555 || gfn == 0x5555555555555555 ) > - { > - orphans_d++; > - //P2M_PRINTK("orphaned guest page: mfn=%#lx has debug gfn\n", > - // mfn); > - continue; > - } > - > - if ( gfn == SHARED_M2P_ENTRY ) > - { > - P2M_PRINTK("shared mfn (%lx) on domain page list!\n", > - mfn); > - continue; > - } > - > - p2mfn = gfn_to_mfn_type_p2m(p2m, gfn, &type, p2m_query); > - if ( strict_m2p && mfn_x(p2mfn) != mfn ) > - { > - mpbad++; > - P2M_PRINTK("map mismatch mfn %#lx -> gfn %#lx -> mfn %#lx" > - " (-> gfn %#lx)\n", > - mfn, gfn, mfn_x(p2mfn), > - (mfn_valid(p2mfn) > - ? get_gpfn_from_mfn(mfn_x(p2mfn)) > - : -1u)); > - /* This m2p entry is stale: the domain has another frame in > - * this physical slot. No great disaster, but for neatness, > - * blow away the m2p entry. */ > - set_gpfn_from_mfn(mfn, INVALID_M2P_ENTRY); > - } > - > - if ( test_linear && (gfn <= p2m->max_mapped_pfn) ) > - { > - lp2mfn = mfn_x(gfn_to_mfn_type_p2m(p2m, gfn, &type, p2m_query)); > - if ( lp2mfn != mfn_x(p2mfn) ) > - { > - P2M_PRINTK("linear mismatch gfn %#lx -> mfn %#lx " > - "(!= mfn %#lx)\n", gfn, lp2mfn, mfn_x(p2mfn)); > - } > - } > - > - // P2M_PRINTK("OK: mfn=%#lx, gfn=%#lx, p2mfn=%#lx, lp2mfn=%#lx\n", > - // mfn, gfn, mfn_x(p2mfn), lp2mfn); > - } > - > - spin_unlock(&d->page_alloc_lock); > - > - /* Audit part two: walk the domain's p2m table, checking the entries. */ > + /* Audit part one: walk the domain's p2m table, checking the entries. */ > if ( pagetable_get_pfn(p2m_get_pagetable(p2m)) != 0 ) > { > l2_pgentry_t *l2e; > @@ -1239,17 +1142,23 @@ void audit_p2m(struct p2m_domain *p2m, i > entry_count); > BUG(); > } > - > - //P2M_PRINTK("p2m audit complete\n"); > - //if ( orphans_i | orphans_d | mpbad | pmbad ) > - // P2M_PRINTK("p2m audit found %lu orphans (%lu inval %lu debug)\n", > - // orphans_i + orphans_d, orphans_i, orphans_d); > - if ( mpbad | pmbad ) > - { > - P2M_PRINTK("p2m audit found %lu odd p2m, %lu bad m2p entries\n", > - pmbad, mpbad); > - WARN(); > - } > + > + return pmbad; > } > #endif /* P2M_AUDIT */ > > +/* Set up the p2m function pointers for pagetable format */ > +void p2m_pt_init(struct p2m_domain *p2m) > +{ > + p2m->set_entry = p2m_set_entry; > + p2m->get_entry = p2m_gfn_to_mfn; > + p2m->change_entry_type_global = p2m_change_type_global; > + p2m->write_p2m_entry = paging_write_p2m_entry; > +#if P2M_AUDIT > + p2m->audit_p2m = p2m_pt_audit_p2m; > +#else > + p2m->audit_p2m = NULL; > +#endif > +} > + > + > diff -r f11528df1df3 -r 764e0872dd4f xen/arch/x86/mm/p2m.c > --- a/xen/arch/x86/mm/p2m.c > +++ b/xen/arch/x86/mm/p2m.c > @@ -439,9 +439,7 @@ guest_physmap_remove_page(struct domain > { > struct p2m_domain *p2m = p2m_get_hostp2m(d); > p2m_lock(p2m); > - audit_p2m(p2m, 1); > p2m_remove_page(p2m, gfn, mfn, page_order); > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > } > > @@ -482,7 +480,6 @@ guest_physmap_add_entry(struct domain *d > return rc; > > p2m_lock(p2m); > - audit_p2m(p2m, 0); > > P2M_DEBUG("adding gfn=%#lx mfn=%#lx\n", gfn, mfn); > > @@ -566,7 +563,6 @@ guest_physmap_add_entry(struct domain *d > } > } > > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > > return rc; > @@ -656,7 +652,6 @@ set_mmio_p2m_entry(struct domain *d, uns > > P2M_DEBUG("set mmio %lx %lx\n", gfn, mfn_x(mfn)); > rc = set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_mmio_direct, > p2m->default_access); > - audit_p2m(p2m, 1); > p2m_unlock(p2m); > if ( 0 == rc ) > gdprintk(XENLOG_ERR, > @@ -688,7 +683,6 @@ clear_mmio_p2m_entry(struct domain *d, u > goto out; > } > rc = set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_4K, > p2m_invalid, p2m->default_access); > - audit_p2m(p2m, 1); > > out: > p2m_unlock(p2m); > @@ -785,7 +779,6 @@ int p2m_mem_paging_nominate(struct domai > > /* Fix p2m entry */ > set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_out, a); > - audit_p2m(p2m, 1); > ret = 0; > > out: > @@ -852,7 +845,6 @@ int p2m_mem_paging_evict(struct domain * > > /* Remove mapping from p2m table */ > set_p2m_entry(p2m, gfn, _mfn(INVALID_MFN), PAGE_ORDER_4K, p2m_ram_paged, > a); > - audit_p2m(p2m, 1); > > /* Clear content before returning the page to Xen */ > scrub_one_page(page); > @@ -946,7 +938,6 @@ void p2m_mem_paging_populate(struct doma > req.flags |= MEM_EVENT_FLAG_EVICT_FAIL; > > set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_in_start, > a); > - audit_p2m(p2m, 1); > } > p2m_unlock(p2m); > > @@ -1014,7 +1005,6 @@ int p2m_mem_paging_prep(struct domain *d > > /* Fix p2m mapping */ > set_p2m_entry(p2m, gfn, mfn, PAGE_ORDER_4K, p2m_ram_paging_in, a); > - audit_p2m(p2m, 1); > > atomic_dec(&d->paged_pages); > > @@ -1063,7 +1053,6 @@ void p2m_mem_paging_resume(struct domain > { > set_p2m_entry(p2m, rsp.gfn, mfn, PAGE_ORDER_4K, p2m_ram_rw, a); > set_gpfn_from_mfn(mfn_x(mfn), rsp.gfn); > - audit_p2m(p2m, 1); > } > p2m_unlock(p2m); > } > @@ -1425,6 +1414,119 @@ unsigned long paging_gva_to_gfn(struct v > return hostmode->gva_to_gfn(v, hostp2m, va, pfec); > } > > +/*** Audit ***/ > + > +#if P2M_AUDIT > +void audit_p2m(struct domain *d, > + uint64_t *orphans_debug, > + uint64_t *orphans_invalid, > + uint64_t *m2p_bad, > + uint64_t *p2m_bad) > +{ > + struct page_info *page; > + struct domain *od; > + unsigned long mfn, gfn; > + mfn_t p2mfn; > + unsigned long orphans_d = 0, orphans_i = 0, mpbad = 0, pmbad = 0; > + p2m_access_t p2ma; > + p2m_type_t type; > + struct p2m_domain *p2m = p2m_get_hostp2m(d); > + > + if ( !paging_mode_translate(d) ) > + goto out_p2m_audit; > + > + P2M_PRINTK("p2m audit starts\n"); > + > + p2m_lock(p2m); > + > + if (p2m->audit_p2m) > + pmbad = p2m->audit_p2m(p2m); > + > + /* Audit part two: walk the domain's page allocation list, checking > + * the m2p entries. */ > + spin_lock(&d->page_alloc_lock); > + page_list_for_each ( page, &d->page_list ) > + { > + mfn = mfn_x(page_to_mfn(page)); > + > + P2M_PRINTK("auditing guest page, mfn=%#lx\n", mfn); > + > + od = page_get_owner(page); > + > + if ( od != d ) > + { > + P2M_PRINTK("wrong owner %#lx -> %p(%u) != %p(%u)\n", > + mfn, od, (od?od->domain_id:-1), d, d->domain_id); > + continue; > + } > + > + gfn = get_gpfn_from_mfn(mfn); > + if ( gfn == INVALID_M2P_ENTRY ) > + { > + orphans_i++; > + P2M_PRINTK("orphaned guest page: mfn=%#lx has invalid gfn\n", > + mfn); > + continue; > + } > + > + if ( gfn == 0x55555555 || gfn == 0x5555555555555555 ) > + { > + orphans_d++; > + P2M_PRINTK("orphaned guest page: mfn=%#lx has debug gfn\n", > + mfn); > + continue; > + } > + > + if ( gfn == SHARED_M2P_ENTRY ) > + { > + P2M_PRINTK("shared mfn (%lx) on domain page list!\n", > + mfn); > + continue; > + } > + > + p2mfn = get_gfn_type_access(p2m, gfn, &type, &p2ma, p2m_query, NULL); > + if ( mfn_x(p2mfn) != mfn ) > + { > + mpbad++; > + P2M_PRINTK("map mismatch mfn %#lx -> gfn %#lx -> mfn %#lx" > + " (-> gfn %#lx)\n", > + mfn, gfn, mfn_x(p2mfn), > + (mfn_valid(p2mfn) > + ? get_gpfn_from_mfn(mfn_x(p2mfn)) > + : -1u)); > + /* This m2p entry is stale: the domain has another frame in > + * this physical slot. No great disaster, but for neatness, > + * blow away the m2p entry. */ > + set_gpfn_from_mfn(mfn, INVALID_M2P_ENTRY); > + } > + __put_gfn(p2m, gfn); > + > + P2M_PRINTK("OK: mfn=%#lx, gfn=%#lx, p2mfn=%#lx, lp2mfn=%#lx\n", > + mfn, gfn, mfn_x(p2mfn), lp2mfn); > + } > + spin_unlock(&d->page_alloc_lock); > + > + p2m_unlock(p2m); > + > + P2M_PRINTK("p2m audit complete\n"); > + if ( orphans_i | orphans_d | mpbad | pmbad ) > + P2M_PRINTK("p2m audit found %lu orphans (%lu inval %lu debug)\n", > + orphans_i + orphans_d, orphans_i, orphans_d); > + if ( mpbad | pmbad ) > + { > + P2M_PRINTK("p2m audit found %lu odd p2m, %lu bad m2p entries\n", > + pmbad, mpbad); > + WARN(); > + } > + > +out_p2m_audit: > + *orphans_debug = (uint64_t) orphans_d; > + *orphans_invalid = (uint64_t) orphans_i; > + *m2p_bad = (uint64_t) mpbad; > + *p2m_bad = (uint64_t) pmbad; > +} > +#endif /* P2M_AUDIT */ > + > /* > * Local variables: > * mode: C > diff -r f11528df1df3 -r 764e0872dd4f xen/include/asm-x86/p2m.h > --- a/xen/include/asm-x86/p2m.h > +++ b/xen/include/asm-x86/p2m.h > @@ -244,6 +244,7 @@ struct p2m_domain { > unsigned long gfn, l1_pgentry_t *p, > mfn_t table_mfn, l1_pgentry_t new, > unsigned int level); > + long (*audit_p2m)(struct p2m_domain *p2m); > > /* Default P2M access type for each page in the the domain: new pages, > * swapped in pages, cleared pages, and pages that are ambiquously > @@ -558,13 +559,15 @@ int set_p2m_entry(struct p2m_domain *p2m > extern void p2m_pt_init(struct p2m_domain *p2m); > > /* Debugging and auditing of the P2M code? */ > -#define P2M_AUDIT 0 > +#define P2M_AUDIT 1 > #define P2M_DEBUGGING 0 > > #if P2M_AUDIT > -extern void audit_p2m(struct p2m_domain *p2m, int strict_m2p); > -#else > -# define audit_p2m(_p2m, _m2p) do { (void)(_p2m),(_m2p); } while (0) > +extern void audit_p2m(struct domain *d, > + uint64_t *orphans_debug, > + uint64_t *orphans_invalid, > + uint64_t *m2p_bad, > + uint64_t *p2m_bad); > #endif /* P2M_AUDIT */ > > /* Printouts */ > diff -r f11528df1df3 -r 764e0872dd4f xen/include/public/domctl.h > --- a/xen/include/public/domctl.h > +++ b/xen/include/public/domctl.h > @@ -800,6 +800,16 @@ struct xen_domctl_mem_sharing_op { > typedef struct xen_domctl_mem_sharing_op xen_domctl_mem_sharing_op_t; > DEFINE_XEN_GUEST_HANDLE(xen_domctl_mem_sharing_op_t); > > +struct xen_domctl_audit_p2m { > + /* OUT error counts */ > + uint64_t orphans_debug; > + uint64_t orphans_invalid; > + uint64_t m2p_bad; > + uint64_t p2m_bad; > +}; > +typedef struct xen_domctl_audit_p2m xen_domctl_audit_p2m_t; > +DEFINE_XEN_GUEST_HANDLE(xen_domctl_audit_p2m_t); > + > #if defined(__i386__) || defined(__x86_64__) > /* XEN_DOMCTL_setvcpuextstate */ > /* XEN_DOMCTL_getvcpuextstate */ > @@ -898,6 +908,7 @@ struct xen_domctl { > #define XEN_DOMCTL_setvcpuextstate 62 > #define XEN_DOMCTL_getvcpuextstate 63 > #define XEN_DOMCTL_set_access_required 64 > +#define XEN_DOMCTL_audit_p2m 65 > #define XEN_DOMCTL_gdbsx_guestmemio 1000 > #define XEN_DOMCTL_gdbsx_pausevcpu 1001 > #define XEN_DOMCTL_gdbsx_unpausevcpu 1002 > @@ -951,6 +962,7 @@ struct xen_domctl { > struct xen_domctl_vcpuextstate vcpuextstate; > #endif > struct xen_domctl_set_access_required access_required; > + struct xen_domctl_audit_p2m audit_p2m; > struct xen_domctl_gdbsx_memio gdbsx_guest_memio; > struct xen_domctl_gdbsx_pauseunp_vcpu gdbsx_pauseunp_vcpu; > struct xen_domctl_gdbsx_domstatus gdbsx_domstatus; > > _______________________________________________ > Xen-devel mailing list > Xen-devel@xxxxxxxxxxxxxxxxxxx > http://lists.xensource.com/xen-devel > _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxxxxxxxx http://lists.xensource.com/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |