Re: [PATCH v2] x86/PoD: defer nested P2M flushes

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Tue, Oct 19, 2021 at 02:52:27PM +0200, Jan Beulich wrote:
> With NPT or shadow in use, the p2m_set_entry() -> p2m_pt_set_entry() ->
> write_p2m_entry() -> p2m_flush_nestedp2m() call sequence triggers a lock
> order violation when the PoD lock is held around it. Hence such flushing
> needs to be deferred. Steal the approach from p2m_change_type_range().
> (Note that strictly speaking the change at the out_of_memory label is
> not needed, as the domain gets crashed there anyway. The change is being
> made nevertheless to avoid setting up a trap from someone meaning to
> deal with that case better than by domain_crash().)
> 
> Similarly for EPT I think ept_set_entry() -> ept_sync_domain() ->
> ept_sync_domain_prepare() -> p2m_flush_nestedp2m() is affected. Make its
> p2m_flush_nestedp2m() invocation conditional. Note that this then also
> alters behavior of p2m_change_type_range() on EPT, deferring the nested
> flushes there as well. I think this should have been that way from the
> introduction of the flag.
> 
> Reported-by: Elliott Mitchell <ehem+xen@xxxxxxx>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

> ---
> v2: Also adjust ept_sync_domain_prepare(). Also convert the flush at the
>     out_of_memory label. Extend description to cover these.
> 
> --- a/xen/arch/x86/mm/p2m-ept.c
> +++ b/xen/arch/x86/mm/p2m-ept.c
> @@ -1253,7 +1253,7 @@ static void ept_sync_domain_prepare(stru
>      {
>          if ( p2m_is_nestedp2m(p2m) )
>              ept = &p2m_get_hostp2m(d)->ept;
> -        else
> +        else if ( !p2m->defer_nested_flush )
>              p2m_flush_nestedp2m(d);

I find this model slightly concerning, as we don't actually notify the
caller that a nested flush as been deferred, so we must make sure that
whoever sets defer_nested_flush also performs a flush unconditionally
when clearing the flag.

Thanks, Roger.