Re: [PATCH 3/4] x86/altp2m: p2m_altp2m_get_or_propagate() should honor present page order

[Andrew Cooper <amc96@xxxxxxxx>]

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