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

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.10.2021 14:59, Roger Pau Monné wrote:
> On Tue, Oct 19, 2021 at 01:58:38PM +0200, Jan Beulich wrote:
>> On 19.10.2021 12:41, Roger Pau Monné wrote:
>>> On Mon, Oct 11, 2021 at 10:17:08AM +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().
>>>>
>>>> Similarly for EPT I think ept_set_entry() -> ept_sync_domain() ->
>>>> ept_sync_domain_prepare() -> p2m_flush_nestedp2m() is affected.
>>>
>>> I'm slightly worried by this path because it doesn't seem to
>>> acknowledge defer_nested_flush.
>>
>> Oh, yes. Iirc I did look at that logic, write down the remark, and
>> then forgot to add the conditional to ept_sync_domain_prepare().
>> The interactions with the real (host) flush are kind of ugly there,
>> though - we then further depend on the ->defer_flush / ->need_flush
>> logic, which is EPT-only. But I think I've convinced myself that
>> this ought to work out.
>>
>>> Maybe the flag should be checked by
>>> p2m_flush_nestedp2m instead of leaving it to the callers?
>>
>> I'm not sure this would be a good idea, as there are more callers.
> 
> We should maybe add an ASSERT to p2m_flush_nestedp2m to make sure it's
> not called with defer_nested_flush being set then, or else it's
> possible for new callers of p2m_flush_nestedp2m to forget checking
> defer_nested_flush.

I'm afraid we can't do that, or at least not this easily: The flag
assumes the p2m lock to be held when it gets set. Hence callers not
holding the lock (hvm_set_efer(), nvmx_handle_invept()) may trigger
such an assert just because another CPU set the flag.

Jan