On 01/12/2021 10:54, Jan Beulich wrote: > @@ -2237,11 +2243,11 @@ bool p2m_altp2m_get_or_propagate(struct > * to the start of the superpage. NB that we repupose `amfn` > * here. > */ > - mask = ~((1UL << page_order) - 1); > + mask = ~((1UL << cur_order) - 1); > amfn = _mfn(mfn_x(*mfn) & mask); > gfn = _gfn(gfn_l & mask); > > - rc = p2m_set_entry(ap2m, gfn, amfn, page_order, *p2mt, *p2ma); > + rc = p2m_set_entry(ap2m, gfn, amfn, cur_order, *p2mt, *p2ma); > p2m_unlock(ap2m);
While I agree with the problem you've identified, this function has some very broken return semantics. Logically, it is taking some hostp2m properties for gfn, and replacing them with ap2m properties for the same gfn. It cannot be correct to only update the caller state on the error paths. At a minimum, the if ( paged ) p2m_mem_paging_populate(currd, _gfn(gfn)); path in the success case is wrong when we've adjusted gfn down. ~Andrew
