Mike Sun
2008-Dec-19 20:54 UTC
[Xen-devel] Making pages writable again in paging_log_dirty mode
Hi, I''m working off the 3.2.x codebase. I''ve seen how "sh_page_fault()" uses "_sh_propagate()" to restore writable access to pages after they have been dirtied. I have a situation in which I''d like to restore writable access to a set of pages that haven''t necessarily been dirtied yet, or have been dirtied, but have not caused a page fault which would then make them writable again. I''m trying to use a hypercall invoked from dom0 that would provide a list of pages to make writable again in log_dirty_mode. Looking at the code which removes all writable mappings to a page from an L2E, I tried to do something like this, where I walk the shadow page tables for the log_dirty_domain and restore writable access to certain pages. Unfortunately, I get errors about page_type_count overflows and other kinds of type errors. SHADOW_FOREACH_L2E(sl2mfn, sl2e, 0, done_l2, v->domain, { flags_l2 = shadow_l2e_get_flags(*sl2e); if ((flags_l2 & _PAGE_PRESENT) && (flags_l2 & _PAGE_USER)) { sl1mfn = shadow_l2e_get_mfn(*sl2e); SHADOW_FOREACH_L1E(sl1mfn, sl1e, 0, done_l1, { flags_l1 = shadow_l1e_get_flags(*sl1e); if ((flags_l1 & _PAGE_PRESENT) && (flags_l1 & _PAGE_USER) && !(flags_l1 & _PAGE_RW)) { mfn = shadow_l1e_get_mfn(*sl1e); if (mfn_valid(mfn)) { struct page_info *page = mfn_to_page(mfn); if ((page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page) { pfn = mfn_to_gfn(v->domain, mfn); if (VALID_M2P(pfn) && my_check(pfn)) { shadow_l1e_t rw_sl1e shadow_l1e_add_flags(*sl1e, _PAGE_RW); (void) shadow_set_l1e(v, sl1e, rw_sl1e, sl1mfn); } } } } }); } }); I dug a bit more to make sure I was increment/decrementing the ref counts correctly and noticed this in the code of "get_page_type()": else if ( unlikely((x & PGT_count_mask) == 0) ) { struct domain *d = page_get_owner(page); /* Never allow a shadowed frame to go from type count 0 to 1 */ if ( d && shadow_mode_enabled(d) ) shadow_remove_all_shadows(d->vcpu[0], _mfn(page_to_mfn(page))); I''m confused about this piece of code? Do we have to destroy all shadows mapping that mfn if the page type changes? Thanks, Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2008-Dec-19 21:08 UTC
[Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> Unfortunately, I get errors about page_type_count > overflows and other kinds of type errors.I forgot to mention that I''m ending up here: int get_page_type(struct page_info *page, unsigned long type) { ... nx = x + 1; if ( unlikely((nx & PGT_count_mask) == 0) ) { MEM_LOG("Type count overflow on pfn %lx", page_to_mfn(page)); return 0; } Not sure how I''m ending up with a -1 type count. I noticed this code in "get_page_from_l1e()": /* Foreign mappings into guests in shadow external mode don''t * contribute to writeable mapping refcounts. (This allows the * qemu-dm helper process in dom0 to map the domain''s memory without * messing up the count of "real" writable mappings.) */ okay = (((l1f & _PAGE_RW) && !(unlikely(paging_mode_external(d) && (d != curr->domain)))) ? get_page_and_type(page, d, PGT_writable_page) : get_page(page, d)); I believe since I''m trying to make writable pages of an HVM guest domain from dom0 in log_dirty_mode, I''m going to end up just doing the get_page() instead of the get_page_and_type(). Is this my problem? Thanks, Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2008-Dec-19 21:59 UTC
[Xen-devel] Re: Making pages writable again in paging_log_dirty mode
Well I''ve managed to figure out the problem. Seems like I was write, if I try to make an HVM guest page writable from dom0 by just changing the PTE on the shadow, it doesn''t increment the page type counter. But it does decrement it, so it overflows. Question then is when pages are made read-only because of log_dirty_mode, are their page types still kept as PGT_writable page (I use that to determine which pages I can make writable again and that shouldn''t be kept as read-only), or do those pages become PGT_none? Thanks! Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-02 10:07 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
At 16:59 -0500 on 19 Dec (1229705974), Mike Sun wrote:> Question then is when pages are made read-only because of > log_dirty_mode, are their page types still kept as PGT_writable page > (I use that to determine which pages I can make writable again and > that shouldn''t be kept as read-only), or do those pages become > PGT_none?IIRC, They should stay as PGT_writeable, with a type count of zero. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-02 16:50 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> IIRC, They should stay as PGT_writeable, with a type count of zero.Thanks a bunch Tim. In tracking down my overflow problem, it lead me to find that that the "get_page_from_l1e()" function has this bit of code that is causing my problem: /* Foreign mappings into guests in shadow external mode don''t * contribute to writeable mapping refcounts. (This allows the * qemu-dm helper process in dom0 to map the domain''s memory without * messing up the count of "real" writable mappings.) */ okay = (((l1f & _PAGE_RW) && !(unlikely(paging_mode_external(d) && (d != curr->domain)))) ? get_page_and_type(page, d, PGT_writable_page) : get_page(page, d)); Basically, I''m trying to make an HVM guest''s pages writable again via a hypercall from dom0, and so this piece of code messes up the type count since I''m actually changing "proper" writeable mapping refcounts. For now, I''m just calling an extra "get_page_and_type(page, d, PGT_writable_page)" manually, and I think it solves this problem, but I''m getting a different kind of kernel panic now where it complains about not being able to "rm_write_access()" due to some special mappings. Any ideas? -- Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-02 17:20 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
Hi, At 11:50 -0500 on 02 Jan (1230897005), Mike Sun wrote:> /* Foreign mappings into guests in shadow external mode don''t > * contribute to writeable mapping refcounts. (This allows the > * qemu-dm helper process in dom0 to map the domain''s memory without > * messing up the count of "real" writable mappings.) */ > okay = (((l1f & _PAGE_RW) && > !(unlikely(paging_mode_external(d) && (d != curr->domain)))) > ? get_page_and_type(page, d, PGT_writable_page) > : get_page(page, d)); > > Basically, I''m trying to make an HVM guest''s pages writable again via > a hypercall from dom0, and so this piece of code messes up the type > count since I''m actually changing "proper" writeable mapping > refcounts.Right; manipulating the shadow from dom0 will make a writeable shadow PTE with no matching typecount and when it''s torn down later the typecount will underflow. :(> For now, I''m just calling an extra > "get_page_and_type(page, d, PGT_writable_page)" manually, and I think > it solves this problem, but I''m getting a different kind of kernel > panic now where it complains about not being able to > "rm_write_access()" due to some special mappings. Any ideas?That''s the opposite problem. The shadow code needs to remove all the writeable mappings of a page (because the guest is using it as a pagetable) and finds that after removing all writeable shadow PTEs that map the page, the typecount has not reached zero. So: - there is some other special reason for the frame to have a typecount. Or, - somehow you''ve removed a writeable shadow entry without decrementing the type count. That shouldn''t be a problem because we know that actions by dom0 (p2m changes, for example) cause writeable shadow PTEs to be torn down (via shadow_put_page_from_l1e) correctly. Or, - you''ve accidentally called get_page_and_type() once too often, creating two typecounts for only one shadow PTE. This is all confused further by the fact that the checks for these un-type-counted mappings in put_page_from_l1e() doesn''t quite match the check in get_page_from_l1e(); ISTR there''s a difference in the semantics of the domain pointer argument to the two functions; maybe Keir can refresh my memory. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Keir Fraser
2009-Jan-02 17:38 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
On 02/01/2009 17:20, "Tim Deegan" <Tim.Deegan@citrix.com> wrote:> This is all confused further by the fact that the checks for these > un-type-counted mappings in put_page_from_l1e() doesn''t quite match the > check in get_page_from_l1e(); ISTR there''s a difference in the semantics > of the domain pointer argument to the two functions; maybe Keir can > refresh my memory.The checks are the same I think, but indeed the domain argument to get_page_from_l1e() and put_page_from_l1e() have slightly different meanings -- the former is essentially a capability, which is checked for permission to create the given PTE (usually the domain which owns the PTE, unless a foreigndom is specified to do_mmu_update()), while the latter is *always* the domain which owns the PTE. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-04 01:43 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> That''s the opposite problem. The shadow code needs to remove all the > writeable mappings of a page (because the guest is using it as a > pagetable) and finds that after removing all writeable shadow PTEs that > map the page, the typecount has not reached zero. So: > - there is some other special reason for the frame to have a > typecount. Or, > - somehow you''ve removed a writeable shadow entry without decrementing > the type count. That shouldn''t be a problem because we know that > actions by dom0 (p2m changes, for example) cause writeable shadow > PTEs to be torn down (via shadow_put_page_from_l1e) correctly. Or, > - you''ve accidentally called get_page_and_type() once too often, > creating two typecounts for only one shadow PTE.Thanks Tim. It make sense that I may accidentally calling get_page_and_type() once too often. I modified the condition in which I manually call an extra get_page_and_type() when trying to make PTEs writable from dom0, but now I get this error condition: (XEN) sh error: sh_remove_shadows(): can''t find all shadows of mfn 02673 (shadow_flags=00000008) I think I''m accounting for the page type refcounts correctly... but I''m probably wrong. Here''s the code that''s trying to add writable mappings back: SHADOW_FOREACH_L1E(sl1mfn, sl1e, 0, done_l1, { flags_l1 = shadow_l1e_get_flags(*sl1e); if ((flags_l1 & _PAGE_PRESENT) && !(flags_l1 & _PAGE_RW)) { mfn = shadow_l1e_get_mfn(*sl1e); pfn = mfn_to_gfn(v->domain, mfn); log_dirty_lock(v->domain); if (mfn_valid(mfn) && VALID_M2P(pfn) && sh_mfn_is_dirty(v->domain, mfn)) { /* hack: because of the external mapping condition * ref count not incremented when this is called from * hypercall originating from dom0, need to do it * manually */ struct page_info *page = mfn_to_page(mfn); if (((page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page) && ((page->u.inuse.type_info & PGT_count_mask) == 0) && get_page_type(page, PGT_writable_page)) { shadow_l1e_t rw_sl1e shadow_l1e_add_flags(*sl1e, _PAGE_RW); shadow_set_l1e(v, sl1e, rw_sl1e, sl1mfn); cow.made_rw_count++; } } log_dirty_unlock(v->domain); } }); Any ideas? Thanks, Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-04 03:55 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> Thanks Tim. It make sense that I may accidentally calling > get_page_and_type() once too often. I modified the condition in which > I manually call an extra get_page_and_type() when trying to make PTEs > writable from dom0, but now I get this error condition: > > (XEN) sh error: sh_remove_shadows(): can''t find all shadows of mfn > 02673 (shadow_flags=00000008)Okay, it turned out to be trickier than I thought to manually call a get_page_and_type(). Seems like I missed a few conditions checks still. I ended up wrapping the get_page_from_l1e and higher functions with another parameter that lets me bypass the external domain mappings not counting code. Thanks for the help! Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-04 23:09 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
>> (XEN) sh error: sh_remove_shadows(): can''t find all shadows of mfn >> 02673 (shadow_flags=00000008)Premature declaration of success. I''m still getting the panic above from this piece of code. SHADOW_FOREACH_L1E(sl1mfn, sl1e, 0, done_l1, { flags_l1 = shadow_l1e_get_flags(*sl1e); if ((flags_l1 & _PAGE_PRESENT) && !(flags_l1 & _PAGE_RW)) { mfn = shadow_l1e_get_mfn(*sl1e); pfn = mfn_to_gfn(v->domain, mfn); log_dirty_lock(v->domain); if (mfn_valid(mfn) && VALID_M2P(pfn) && sh_mfn_is_dirty(v->domain, mfn)) { /* hack: because of the external mapping condition * ref count not incremented when this is called from * hypercall originating from dom0, need to do it * manually */ struct page_info *page = mfn_to_page(mfn); if (((page->u.inuse.type_info & PGT_type_mask) == PGT_writable_page) && ((page->u.inuse.type_info & PGT_count_mask) == 0) && get_page_type(page, PGT_writable_page)) { shadow_l1e_t rw_sl1e shadow_l1e_add_flags(*sl1e, _PAGE_RW); shadow_write_entries(sl1e, &rw_sl1e, 1, sl1mfn); cow.made_rw_count++; } } log_dirty_unlock(v->domain); } }); Am I failing to update the page count somewhere? Or are there more conditions I need to check for the page that I''m adding a RW mapping to? Thanks, Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-05 10:11 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
Hi, At 18:09 -0500 on 04 Jan (1231092582), Mike Sun wrote:> >> (XEN) sh error: sh_remove_shadows(): can''t find all shadows of mfn > >> 02673 (shadow_flags=00000008) > > Premature declaration of success. I''m still getting the panic above > from this piece of code. > > SHADOW_FOREACH_L1E(sl1mfn, sl1e, 0, done_l1, > { > flags_l1 = shadow_l1e_get_flags(*sl1e); > if ((flags_l1 & _PAGE_PRESENT) && !(flags_l1 & _PAGE_RW)) > { > mfn = shadow_l1e_get_mfn(*sl1e); > pfn = mfn_to_gfn(v->domain, mfn); > log_dirty_lock(v->domain); > if (mfn_valid(mfn) && VALID_M2P(pfn) && > sh_mfn_is_dirty(v->domain, mfn)) > { > /* hack: because of the external mapping condition > * ref count not incremented when this is called from > * hypercall originating from dom0, need to do it > * manually > */ > struct page_info *page = mfn_to_page(mfn); > if (((page->u.inuse.type_info & PGT_type_mask) > == PGT_writable_page) > && ((page->u.inuse.type_info & PGT_count_mask) == 0)That looks quite strange to me; it will select every dirty page that is not currently writeable - regardless of whether the guest asked for a writeable mapping (e.g. user-mode read-only maps of kernel structures); and - regardless of whether it''s currently shadowed! If you make a shadowed page writeable this way then the backstop check in get_page_type() (which you asked about in another email) will force it to be unshadowed immediately. If you do that to the currently in-use top-level pagetable, you''ll get the crash above. I think you need to check for !sh_mfn_is_a_page_table(mfn) before trying to make this writeable.> && get_page_type(page, PGT_writable_page)) > { > shadow_l1e_t rw_sl1e > shadow_l1e_add_flags(*sl1e, _PAGE_RW);Did you by any chance make this shadow_l1e_add_flags() operator by copying l1e_add_flags from asm/page.h? That''s a macro which modifies its arguments, which you are definitely not allowed to do to a live shadow pagetable. :) Cheers, Tim.> shadow_write_entries(sl1e, &rw_sl1e, 1, sl1mfn); > cow.made_rw_count++; > } > } > log_dirty_unlock(v->domain); > } > }); > > Am I failing to update the page count somewhere? Or are there more > conditions I need to check for the page that I''m adding a RW mapping > to? > > Thanks, > Mike-- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-05 14:24 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> That looks quite strange to me; it will select every dirty page that is > not currently writeableAh yes, I should probably explain what I''m doing. I''m using the log-dirty mode to catch any writes to pages that I haven''t yet "copied" via a background copy mechanism, similar to the live migration copy. If I have copied it already, then I manually mark it dirty to indicate that it can be made writable again. That''s what I''m trying to do in this code.> - regardless of whether the guest asked for a writeable mapping > (e.g. user-mode read-only maps of kernel structures); and > - regardless of whether it''s currently shadowed!Yeah, I''ve tried adding checks as well (not seen in this version of the code) to first check the guest PTE to see if it''s writable and to check to see if it''s shadowed. But I also thought that if I check that that mapped page type was PGT_writable_page with a type count of 0, I could assume that the guest PTE mapped the page as writable, no?>> && get_page_type(page, PGT_writable_page)) >> { >> shadow_l1e_t rw_sl1e >> shadow_l1e_add_flags(*sl1e, _PAGE_RW); > > Did you by any chance make this shadow_l1e_add_flags() operator by > copying l1e_add_flags from asm/page.h? That''s a macro which modifies > its arguments, which you are definitely not allowed to do to a live > shadow pagetable. :)Now this is probably a big problem, that''s exactly what I''m doing and I didn''t think about that! I''ll fix that and see what happens. Thanks for all the help! Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-05 14:39 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
At 09:24 -0500 on 05 Jan (1231147482), Mike Sun wrote:> Yeah, I''ve tried adding checks as well (not seen in this version of > the code) to first check the guest PTE to see if it''s writable and to > check to see if it''s shadowed. But I also thought that if I check > that that mapped page type was PGT_writable_page with a type count of > 0, I could assume that the guest PTE mapped the page as writable, no?No, sorry. All it means is that the last typecount it has was PGT_writeable -- which will indeed have come from a writeable PTE, but it might not have been this one! The page could have been freed and recycled as a copy-on-write text page (and dirtied by DMA filling it with data); or it could be writeable in the kernel and this PTE could be a read-only user-mode mapping of the same data... And even if this guest PTE allows write access, you can''t allow write access in the shadow PTE if the target mfn is shadowed, because all shadowed pages must be read-only in the shadows so that we can intercept and emulate writes to them. So you need to check that too. Ideally you''d replicate all the checks that are done in _sh_propagate (guest PTE allows writes, target pfn not read-only, target mfn not shadowed, maybe some others I don''t remember). Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Mike Sun
2009-Jan-05 14:54 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
> Ideally you''d replicate all the checks that are done in _sh_propagate > (guest PTE allows writes, target pfn not read-only, target mfn not > shadowed, maybe some others I don''t remember).For ease of correctness, would using something like validate_g1le() make all the checks I need (since it calls _sh_propagate down the line), assuming I call this while in the context of the same domain as v->domain: SHADOW_FOREACH_L2E(sl2mfn, sl2e, 0, done_l2, v->domain, { flags_l2 = shadow_l2e_get_flags(*sl2e); if (flags_l2 & _PAGE_PRESENT) { sl1mfn = shadow_l2e_get_mfn(*sl2e); gl1mfn = _mfn(mfn_to_shadow_page(sl1mfn)->backpointer); gl1e = gp = sh_map_domain_page(gl1mfn); SHADOW_FOREACH_L1E(sl1mfn, sl1e, &gl1e, done_l1, { flags_l1 = shadow_l1e_get_flags(*sl1e); if ((flags_l1 & _PAGE_PRESENT) && !(flags_l1 & _PAGE_RW)) { mfn = shadow_l1e_get_mfn(*sl1e); pfn = mfn_to_gfn(v->domain, mfn); log_dirty_lock(v->domain); if (mfn_valid(mfn) && VALID_M2P(pfn) && sh_mfn_is_dirty(v->domain, mfn)) { if (guest_l1e_get_flags(*gl1e) & _PAGE_RW) validate_gl1e(v, gl1e, sl1mfn, sl1e); } log_dirty_unlock(v->domain); } }); } }); Thanks, Mike _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-05 15:00 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
At 09:54 -0500 on 05 Jan (1231149240), Mike Sun wrote:> For ease of correctness, would using something like validate_g1le() > make all the checks I need (since it calls _sh_propagate down the > line), assuming I call this while in the context of the same domain as > v->domain:Yes, I think so. The page is already dirty so validate_gl1e should DTRT, if not necessarily the fastest way. Of course, if you call it from dom0 you''ll end up with the same problem you had before, of get_page_from_l1e() not refcounting correctly. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Tim Deegan
2009-Jan-05 15:07 UTC
Re: [Xen-devel] Re: Making pages writable again in paging_log_dirty mode
At 09:24 -0500 on 05 Jan (1231147482), Mike Sun wrote:> Ah yes, I should probably explain what I''m doing. I''m using the > log-dirty mode to catch any writes to pages that I haven''t yet > "copied" via a background copy mechanism, similar to the live > migration copy. If I have copied it already, then I manually mark it > dirty to indicate that it can be made writable again. That''s what I''m > trying to do in this code.Erm, did you know that if you mark the page as dirty the normal page-fault path will fix up the shadow PTEs the next time they cause a write fault? Not as fast as fixing them all in bulk (a few thousand cycles for each pagefault and VMEXIT) but should be functionally correct. Cheers, Tim. -- Tim Deegan <Tim.Deegan@citrix.com> Principal Software Engineer, Citrix Systems (R&D) Ltd. [Company #02300071, SL9 0DZ, UK.] _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel