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

[Paul Durrant <xadimgnik@xxxxxxxxx>]

> -----Original Message-----
> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of Tamas K 
> Lengyel
> Sent: 01 June 2020 14:22
> To: xen-devel@xxxxxxxxxxxxxxxxxxxx
> Cc: Kevin Tian <kevin.tian@xxxxxxxxx>; Stefano Stabellini 
> <sstabellini@xxxxxxxxxx>; Tamas K Lengyel
> <tamas.lengyel@xxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Wei Liu 
> <wl@xxxxxxx>; Andrew Cooper
> <andrew.cooper3@xxxxxxxxxx>; Ian Jackson <ian.jackson@xxxxxxxxxxxxx>; George 
> Dunlap
> <george.dunlap@xxxxxxxxxx>; Tamas K Lengyel <tamas@xxxxxxxxxxxxx>; Jan 
> Beulich <jbeulich@xxxxxxxx>;
> Julien Grall <julien@xxxxxxx>; Roger Pau Monné <roger.pau@xxxxxxxxxx>
> Subject: [PATCH v19 for-4.14 01/13] x86/mem_sharing: block interrupt 
> injection for forks
> 
> 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
> 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>

Release-acked-by: Paul Durrant <paul@xxxxxxx>

> ---
>  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
>