RE: [PATCH v19 for-4.14 01/13] x86/mem_sharing: block interrupt injection for forks

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Lengyel, Tamas <tamas.lengyel@xxxxxxxxx>
> Sent: Monday, June 1, 2020 9:22 PM
> 
> When running VM forks without device models (QEMU), it may
> be undesirable for Xen to inject interrupts. When creating such forks from
> Windows VMs we have observed the kernel trying to process interrupts
> immediately after the fork is executed. However without QEMU running such
> interrupt handling may not be possible because it may attempt to interact
> with
> devices that are not emulated by a backend. In the best case scenario such

I asked this question before. the interrupts could come from Xen itself, e.g.
due to timer virtualization. So I didn't get why it's desired to block all 
interrupts
just because no QEMU is running. Also it's weird why Windows VMs are
observed to process interrupts that are generated by QEMU when no such
backend emulation exists at all. It sounds like a workaround instead of a real
fix...


> interrupt handling would only present a detour in the VM forks' execution
> flow, but in the worst case as we actually observed can completely stall it.
> By disabling interrupt injection a fuzzer can exercise the target code without
> interference. For other use-cases this option probably doesn't make sense,
> that's why this is not enabled by default.
> 
> Forks & memory sharing are only available on Intel CPUs so this only applies
> to vmx. Note that this is part of the experimental VM forking feature that's
> completely disabled by default and can only be enabled by using
> XEN_CONFIG_EXPERT during compile time.
> 
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/x86/hvm/vmx/intr.c      | 6 ++++++
>  xen/arch/x86/mm/mem_sharing.c    | 6 +++++-
>  xen/include/asm-x86/hvm/domain.h | 2 +-
>  xen/include/public/memory.h      | 3 +++
>  4 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/xen/arch/x86/hvm/vmx/intr.c b/xen/arch/x86/hvm/vmx/intr.c
> index 000e14af49..80bfbb4787 100644
> --- 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
> +
>      /* Crank the handle on interrupt state. */
>      pt_vector = pt_update_irq(v);
> 
> diff --git a/xen/arch/x86/mm/mem_sharing.c
> b/xen/arch/x86/mm/mem_sharing.c
> index 19922ab5d1..c428fd16ce 100644
> --- a/xen/arch/x86/mm/mem_sharing.c
> +++ b/xen/arch/x86/mm/mem_sharing.c
> @@ -2106,7 +2106,8 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op
> _t) arg)
>          rc = -EINVAL;
>          if ( mso.u.fork.pad )
>              goto out;
> -        if ( mso.u.fork.flags & ~XENMEM_FORK_WITH_IOMMU_ALLOWED )
> +        if ( mso.u.fork.flags &
> +             ~(XENMEM_FORK_WITH_IOMMU_ALLOWED |
> XENMEM_FORK_BLOCK_INTERRUPTS) )
>              goto out;
> 
>          rc = rcu_lock_live_remote_domain_by_id(mso.u.fork.parent_domain,
> @@ -2134,6 +2135,9 @@ int
> mem_sharing_memop(XEN_GUEST_HANDLE_PARAM(xen_mem_sharing_op
> _t) arg)
>              rc = hypercall_create_continuation(__HYPERVISOR_memory_op,
>                                                 "lh", XENMEM_sharing_op,
>                                                 arg);
> +        else if ( !rc && (mso.u.fork.flags &
> XENMEM_FORK_BLOCK_INTERRUPTS) )
> +            d->arch.hvm.mem_sharing.block_interrupts = true;
> +
>          rcu_unlock_domain(pd);
>          break;
>      }
> diff --git a/xen/include/asm-x86/hvm/domain.h b/xen/include/asm-
> x86/hvm/domain.h
> index 95fe18cddc..9d247baf4d 100644
> --- a/xen/include/asm-x86/hvm/domain.h
> +++ b/xen/include/asm-x86/hvm/domain.h
> @@ -67,7 +67,7 @@ struct hvm_ioreq_server {
>  #ifdef CONFIG_MEM_SHARING
>  struct mem_sharing_domain
>  {
> -    bool enabled;
> +    bool enabled, block_interrupts;
> 
>      /*
>       * When releasing shared gfn's in a preemptible manner, recall where
> diff --git a/xen/include/public/memory.h b/xen/include/public/memory.h
> index dbd35305df..850bd72c52 100644
> --- a/xen/include/public/memory.h
> +++ b/xen/include/public/memory.h
> @@ -536,7 +536,10 @@ struct xen_mem_sharing_op {
>          } debug;
>          struct mem_sharing_op_fork {      /* OP_FORK */
>              domid_t parent_domain;        /* IN: parent's domain id */
> +/* Only makes sense for short-lived forks */
>  #define XENMEM_FORK_WITH_IOMMU_ALLOWED (1u << 0)
> +/* Only makes sense for short-lived forks */
> +#define XENMEM_FORK_BLOCK_INTERRUPTS   (1u << 1)
>              uint16_t flags;               /* IN: optional settings */
>              uint32_t pad;                 /* Must be set to 0 */
>          } fork;
> --
> 2.25.1