| Message ID | a7ecf7703357130dbd9f23481d39adafea569872.1591017086.git.tamas.lengyel@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | VM forking | expand |
> -----Original Message----- > From: Xen-devel <xen-devel-bounces@lists.xenproject.org> On Behalf Of Tamas K Lengyel > Sent: 01 June 2020 14:22 > To: xen-devel@lists.xenproject.org > Cc: Kevin Tian <kevin.tian@intel.com>; Stefano Stabellini <sstabellini@kernel.org>; Tamas K Lengyel > <tamas.lengyel@intel.com>; Jun Nakajima <jun.nakajima@intel.com>; Wei Liu <wl@xen.org>; Andrew Cooper > <andrew.cooper3@citrix.com>; Ian Jackson <ian.jackson@eu.citrix.com>; George Dunlap > <george.dunlap@citrix.com>; Tamas K Lengyel <tamas@tklengyel.com>; Jan Beulich <jbeulich@suse.com>; > Julien Grall <julien@xen.org>; Roger Pau Monné <roger.pau@citrix.com> > 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@intel.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Release-acked-by: Paul Durrant <paul@xen.org> > --- > 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 >
On Mon, Jun 01, 2020 at 06:21:35AM -0700, Tamas K Lengyel wrote: > 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@intel.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> Reviewed-by: Wei Liu <wl@xen.org>
> From: Lengyel, Tamas <tamas.lengyel@intel.com> > 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@intel.com> > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > --- > 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
> From: Tian, Kevin > Sent: Wednesday, June 10, 2020 7:44 AM > > > From: Lengyel, Tamas <tamas.lengyel@intel.com> > > 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... ok, I rechecked your reply. Looks it's about the case that parent VM has QEMU and pending interrupts while you fork it into child VMs without QEMU so those pending interrupts become problematic. Reviewed-by: Kevin Tian <kevin.tian@intel.com> > > > > 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@intel.com> > > Reviewed-by: Roger Pau Monné <roger.pau@citrix.com> > > --- > > 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
On Tue, Jun 9, 2020 at 5:53 PM Tian, Kevin <kevin.tian@intel.com> wrote: > > > From: Tian, Kevin > > Sent: Wednesday, June 10, 2020 7:44 AM > > > > > From: Lengyel, Tamas <tamas.lengyel@intel.com> > > > 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... > > ok, I rechecked your reply. Looks it's about the case that parent VM has QEMU > and pending interrupts while you fork it into child VMs without QEMU so those > pending interrupts become problematic. > > Reviewed-by: Kevin Tian <kevin.tian@intel.com> HI Kevin, thanks! That's the case but really we just want to block all interrupts irrespective of where the are coming from. The fork VMs are being reset hundreds or thousands of times per second during fuzzing, so there is no point in injecting any interrupt at all in that particular use-case. Tamas
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;