[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [RFC PATCH] hvm/vpt: fix inconsistent views of vIOAPIC in vmx_intr_assist()
> From: Gao, Chao > Sent: Friday, April 7, 2017 11:24 AM > > When injecting periodic timer interrupt in vmx_intr_assist(), multiple read > operation is operated during one event delivery and incur to inconsistent operation is operated -> operations are done > views of vIOAPIC. For example, if a periodic timer interrupt is from PIT, when > set the corresponding bit in vIRR, the corresponding RTE is accessed in > pt_update_irq(). When this function returns, it accesses the RTE again to get > the vector it set in vIRR. Between the two accesses, the content of RTE may > have been changed by another CPU for no protection method in use. This > case > can incur the assertion failure in vmx_intr_assist(). Also after a periodic > timer interrupt is injected successfully, pt_irq_posted() will decrease the > count (pending_intr_nr). But if the corresponding RTE has been changed, > pt_irq_posted() will not decrease the count, resulting one more timer > interrupt. > > More discussion and history can be found in > 1.https://lists.xenproject.org/archives/html/xen-devel/2017- > 03/msg00906.html > 2.https://lists.xenproject.org/archives/html/xen-devel/2017- > 01/msg01019.html > > This patch simply uses irq_lock to avoid these inconsistent views of vIOAPIC. > To fix the deadlock, provide locked version of several functions. > > Signed-off-by: Chao Gao <chao.gao@xxxxxxxxx> > --- > xen/arch/x86/hvm/irq.c | 22 ++++++++++++++++------ > xen/arch/x86/hvm/svm/intr.c | 21 +++++++++++++++------ > xen/arch/x86/hvm/vmx/intr.c | 9 +++++++++ > xen/arch/x86/hvm/vpic.c | 20 ++++++++++++++------ > xen/arch/x86/hvm/vpt.c | 4 ++-- > xen/include/xen/hvm/irq.h | 4 ++++ > 6 files changed, 60 insertions(+), 20 deletions(-) > [...] > diff --git a/xen/arch/x86/hvm/vpic.c b/xen/arch/x86/hvm/vpic.c > index e160bbd..8d02e52 100644 > --- a/xen/arch/x86/hvm/vpic.c > +++ b/xen/arch/x86/hvm/vpic.c > @@ -153,14 +153,13 @@ static void __vpic_intack(struct hvm_hw_vpic *vpic, > int irq) > vpic_update_int_output(vpic); > } > > -static int vpic_intack(struct hvm_hw_vpic *vpic) > +static int vpic_intack_locked(struct hvm_hw_vpic *vpic) > { > int irq = -1; > > - vpic_lock(vpic); > - > + ASSERT(vpic_is_locked(vpic)); > if ( !vpic->int_output ) > - goto out; > + return irq; > > irq = vpic_get_highest_priority_irq(vpic); > BUG_ON(irq < 0); > @@ -175,7 +174,15 @@ static int vpic_intack(struct hvm_hw_vpic *vpic) > irq += 8; > } > > - out: > + return irq; > +} > + > +static int vpic_intack(struct hvm_hw_vpic *vpic) > +{ > + int irq; > + > + vpic_lock(vpic); > + irq = vpic_intack_locked(vpic); > vpic_unlock(vpic); > return irq; > } > @@ -487,13 +494,14 @@ int vpic_ack_pending_irq(struct vcpu *v) > struct hvm_hw_vpic *vpic = &v->domain->arch.hvm_domain.vpic[0]; > > ASSERT(has_vpic(v->domain)); > + ASSERT(vpic_is_locked(vpic)); > > TRACE_2D(TRC_HVM_EMUL_PIC_PEND_IRQ_CALL, > vlapic_accept_pic_intr(v), > vpic->int_output); > if ( !vlapic_accept_pic_intr(v) || !vpic->int_output ) > return -1; > > - irq = vpic_intack(vpic); > + irq = vpic_intack_locked(vpic); > if ( irq == -1 ) > return -1; > hvm_vcpu_ack_pending_irq is also invoked in nvmx_intr_intercept, where you also need surround it with spin_lock/unlock otherwise above ASSERT will be triggered. Thanks Kevin _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |