In preparation for reactivating the presently dead 2M page path of the function, - also deal with the case of replacing an L1 page table all in one go, - pull common checks out of the switch(). This includes extending a _PAGE_PRESENT check to L1 as well, which presumably was deemed redundant with p2m_is_valid() || p2m_is_grant(), but I think we are better off being explicit in all cases, - replace a p2m_is_ram() check in the 2M case by an explicit _PAGE_PRESENT one, to make more obvious that the subsequent l1e_get_mfn() actually retrieves something that is actually an MFN.
Signed-off-by: Jan Beulich <[email protected]> --- a/xen/arch/x86/mm/shadow/hvm.c +++ b/xen/arch/x86/mm/shadow/hvm.c @@ -801,7 +801,7 @@ static void sh_unshadow_for_p2m_change(s l1_pgentry_t old, l1_pgentry_t new, unsigned int level) { - mfn_t omfn = l1e_get_mfn(old); + mfn_t omfn = l1e_get_mfn(old), nmfn; unsigned int oflags = l1e_get_flags(old); p2m_type_t p2mt = p2m_flags_to_type(oflags); bool flush = false; @@ -813,19 +813,30 @@ static void sh_unshadow_for_p2m_change(s if ( unlikely(!d->arch.paging.shadow.total_pages) ) return; + /* Only previously present / valid entries need processing. */ + if ( !(oflags & _PAGE_PRESENT) || + (!p2m_is_valid(p2mt) && !p2m_is_grant(p2mt)) || + !mfn_valid(omfn) ) + return; + + nmfn = l1e_get_flags(new) & _PAGE_PRESENT ? l1e_get_mfn(new) : INVALID_MFN; + switch ( level ) { default: /* * The following assertion is to make sure we don't step on 1GB host - * page support of HVM guest. + * page support of HVM guest. Plus we rely on ->set_entry() to never + * get called with orders above PAGE_ORDER_2M, not even to install + * non-present entries (which in principle ought to be fine even + * without respective large page support). */ - ASSERT(!((oflags & _PAGE_PRESENT) && (oflags & _PAGE_PSE))); + ASSERT_UNREACHABLE(); break; /* If we're removing an MFN from the p2m, remove it from the shadows too */ case 1: - if ( (p2m_is_valid(p2mt) || p2m_is_grant(p2mt)) && mfn_valid(omfn) ) + if ( !mfn_eq(nmfn, omfn) ) { sh_remove_all_shadows_and_parents(d, omfn); if ( sh_remove_all_mappings(d, omfn, _gfn(gfn)) ) @@ -839,14 +850,9 @@ static void sh_unshadow_for_p2m_change(s * scheme, that's OK, but otherwise they must be unshadowed. */ case 2: - if ( !(oflags & _PAGE_PRESENT) || !(oflags & _PAGE_PSE) ) - break; - - if ( p2m_is_valid(p2mt) && mfn_valid(omfn) ) { unsigned int i; - mfn_t nmfn = l1e_get_mfn(new); - l1_pgentry_t *npte = NULL; + l1_pgentry_t *npte = NULL, *opte = NULL; /* If we're replacing a superpage with a normal L1 page, map it */ if ( (l1e_get_flags(new) & _PAGE_PRESENT) && @@ -854,24 +860,39 @@ static void sh_unshadow_for_p2m_change(s mfn_valid(nmfn) ) npte = map_domain_page(nmfn); + /* If we're replacing a normal L1 page, map it as well. */ + if ( !(oflags & _PAGE_PSE) ) + opte = map_domain_page(omfn); + gfn &= ~(L1_PAGETABLE_ENTRIES - 1); for ( i = 0; i < L1_PAGETABLE_ENTRIES; i++ ) { - if ( !npte || - !p2m_is_ram(p2m_flags_to_type(l1e_get_flags(npte[i]))) || - !mfn_eq(l1e_get_mfn(npte[i]), omfn) ) + if ( opte ) + { + if ( !(l1e_get_flags(opte[i]) & _PAGE_PRESENT) ) + continue; + omfn = l1e_get_mfn(opte[i]); + } + + if ( npte ) + nmfn = l1e_get_flags(npte[i]) & _PAGE_PRESENT + ? l1e_get_mfn(npte[i]) : INVALID_MFN; + + if ( !mfn_eq(nmfn, omfn) ) { /* This GFN->MFN mapping has gone away */ sh_remove_all_shadows_and_parents(d, omfn); if ( sh_remove_all_mappings(d, omfn, _gfn(gfn + i)) ) flush = true; } + omfn = mfn_add(omfn, 1); + nmfn = mfn_add(nmfn, 1); } - if ( npte ) - unmap_domain_page(npte); + unmap_domain_page(opte); + unmap_domain_page(npte); } break;
