| Message ID | 76cc0b4a-27ca-21b7-841f-315f31833762@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/shadow: misc tidying | expand |
On 05/01/2023 4:04 pm, Jan Beulich wrote: > Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm: > pagetable_dying() is HVM-only"). Convert both to assertions, noting that > in particular the one in the 3-level variant of the function comes too "came too late"? It doesn't any more with this change in place. > late anyway - first thing there we access the HVM part of a union. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Andrew Cooper <andrew.cooper3@citrix.com>
On 06.01.2023 02:00, Andrew Cooper wrote: > On 05/01/2023 4:04 pm, Jan Beulich wrote: >> Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm: >> pagetable_dying() is HVM-only"). Convert both to assertions, noting that >> in particular the one in the 3-level variant of the function comes too > > "came too late"? > > It doesn't any more with this change in place. Fine with me either way, so I've changed it. Iirc in particular George has been advocating for writing descriptions in present tense, even if in the course of a change the stated fact changes as well. >> late anyway - first thing there we access the HVM part of a union. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Andrew Cooper <andrew.cooper3@citrix.com> Thanks. Jan
--- a/xen/arch/x86/mm/shadow/multi.c +++ b/xen/arch/x86/mm/shadow/multi.c @@ -3780,6 +3780,8 @@ static void cf_check sh_pagetable_dying( unsigned long l3gfn; mfn_t l3mfn; + ASSERT(is_hvm_domain(d)); + gcr3 = v->arch.hvm.guest_cr[3]; /* fast path: the pagetable belongs to the current context */ if ( gcr3 == gpa ) @@ -3822,7 +3824,7 @@ static void cf_check sh_pagetable_dying( : shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l2_pae_shadow); } - if ( mfn_valid(smfn) && is_hvm_domain(d) ) + if ( mfn_valid(smfn) ) { gmfn = _mfn(mfn_to_page(smfn)->v.sh.back); mfn_to_page(gmfn)->pagetable_dying = true; @@ -3854,6 +3856,8 @@ static void cf_check sh_pagetable_dying( mfn_t smfn, gmfn; p2m_type_t p2mt; + ASSERT(is_hvm_domain(d)); + gmfn = get_gfn_query(d, _gfn(gpa >> PAGE_SHIFT), &p2mt); paging_lock(d); @@ -3863,7 +3867,7 @@ static void cf_check sh_pagetable_dying( smfn = shadow_hash_lookup(d, mfn_x(gmfn), SH_type_l4_64_shadow); #endif - if ( mfn_valid(smfn) && is_hvm_domain(d) ) + if ( mfn_valid(smfn) ) { mfn_to_page(gmfn)->pagetable_dying = true; shadow_unhook_mappings(d, smfn, 1/* user pages only */);
Perhaps these should have been dropped right in 2fb2dee1ac62 ("x86/mm: pagetable_dying() is HVM-only"). Convert both to assertions, noting that in particular the one in the 3-level variant of the function comes too late anyway - first thing there we access the HVM part of a union. Signed-off-by: Jan Beulich <jbeulich@suse.com>