Re: [PATCH v2 for-4.14 1/2] x86/mem_sharing: block interrupt injection for forks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 22.05.2020 18:33, Tamas K Lengyel wrote:
> When running shallow forks without device models it may be undesirable for Xen
> to inject interrupts. With Windows forks we have observed the kernel going 
> into
> infinite loops when trying to process such interrupts, likely because it 
> attempts
> to interact with devices that are not responding without QEMU running. By
> disabling interrupt injection the fuzzer can exercise the target code without
> interference.
> 
> Forks & memory sharing are only available on Intel CPUs so this only applies
> to vmx.

Looking at e.g. mem_sharing_control() I can't seem to be able to confirm
this. Would you mind pointing me at where this restriction is coming from?

> --- a/xen/arch/x86/hvm/vmx/intr.c
> +++ b/xen/arch/x86/hvm/vmx/intr.c
> @@ -256,6 +256,12 @@ void vmx_intr_assist(void)
>      if ( unlikely(v->arch.vm_event) && v->arch.vm_event->sync_event )
>          return;
>  
> +#ifdef CONFIG_MEM_SHARING
> +    /* Block event injection for VM fork if requested */
> +    if ( unlikely(v->domain->arch.hvm.mem_sharing.block_interrupts) )
> +        return;
> +#endif

The two earlier returns are temporary as far as the guest is concerned,
i.e. eventually the interrupt(s) will get delivered. The one you add
looks as if it is a permanent thing, i.e. interrupt requests will pile
up and potentially confuse a guest down the road. This _may_ be okay
for your short-lived-shallow-fork scenario, but then wants at least
calling out in the public header by a comment (and I think the same
goes for XENMEM_FORK_WITH_IOMMU_ALLOWED that's already there).

> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -74,6 +74,8 @@ struct mem_sharing_domain
>       * to resume the search.
>       */
>      unsigned long next_shared_gfn_to_relinquish;
> +
> +    bool block_interrupts;
>  };

Please can you avoid unnecessary growth of the structure by inserting
next to the pre-existing bool rather than at the end?

Jan