| Message ID | 1484233120-2015-9-git-send-email-paul.durrant@citrix.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 12/01/17 14:58, Paul Durrant wrote: > Since injection works on a remote vCPU, and since there's no > enforcement of the subject vCPU being paused, there's a potential race > between the producing and consuming sides. Fix this by leveraging the > vector field as synchronization variable. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > [re-based] > Signed-off-by: Paul Durrant <paul.durrant@citrix.com> > --- > Cc: Andrew Cooper <andrew.cooper3@citrix.com> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> This looks fairly unrelated to the other dm changes. Given that it is a backport candidate, should it be pulled ahead of the move to dm.c? (I can make this happen on commit if we are in agreement).
>>> On 12.01.17 at 17:28, <andrew.cooper3@citrix.com> wrote: > On 12/01/17 14:58, Paul Durrant wrote: >> Since injection works on a remote vCPU, and since there's no >> enforcement of the subject vCPU being paused, there's a potential race >> between the producing and consuming sides. Fix this by leveraging the >> vector field as synchronization variable. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> [re-based] >> Signed-off-by: Paul Durrant <paul.durrant@citrix.com> >> --- >> Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> > > This looks fairly unrelated to the other dm changes. Given that it is a > backport candidate, should it be pulled ahead of the move to dm.c? (I > can make this happen on commit if we are in agreement). This indeed was the case from the very beginning of the HVMOP series, and I've never done the re-order because I had never expected it to take so long for the earlier patches to go in. And admittedly I was also too lazy (or should I say too busy, to make it sound better) to do that re-ordering, as the fix didn't seem important enough to bother (I'm not sure why you think this would be a backporting candidate, as I don't think this hypercall is used by anything that's in fully supported state). Jan
diff --git a/xen/arch/x86/hvm/dm.c b/xen/arch/x86/hvm/dm.c index 0a7e50a..e09c8e7 100644 --- a/xen/arch/x86/hvm/dm.c +++ b/xen/arch/x86/hvm/dm.c @@ -286,13 +286,16 @@ static int inject_trap(struct domain *d, unsigned int vcpuid, if ( vcpuid >= d->max_vcpus || !(v = d->vcpu[vcpuid]) ) return -EINVAL; - if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ) + if ( cmpxchg(&v->arch.hvm_vcpu.inject_trap.vector, + HVM_TRAP_VECTOR_UNSET, HVM_TRAP_VECTOR_UPDATING) != + HVM_TRAP_VECTOR_UNSET ) return -EBUSY; v->arch.hvm_vcpu.inject_trap.type = type; v->arch.hvm_vcpu.inject_trap.insn_len = insn_len; v->arch.hvm_vcpu.inject_trap.error_code = error_code; v->arch.hvm_vcpu.inject_trap.cr2 = cr2; + smp_wmb(); v->arch.hvm_vcpu.inject_trap.vector = vector; return 0; diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c index 8d42adc..441111b 100644 --- a/xen/arch/x86/hvm/hvm.c +++ b/xen/arch/x86/hvm/hvm.c @@ -539,12 +539,14 @@ void hvm_do_resume(struct vcpu *v) } /* Inject pending hw/sw trap */ - if ( v->arch.hvm_vcpu.inject_trap.vector != -1 ) + if ( v->arch.hvm_vcpu.inject_trap.vector >= 0 ) { + smp_rmb(); + if ( !hvm_event_pending(v) ) hvm_inject_event(&v->arch.hvm_vcpu.inject_trap); - v->arch.hvm_vcpu.inject_trap.vector = -1; + v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET; } if ( unlikely(v->arch.vm_event) && v->arch.monitor.next_interrupt_enabled ) @@ -1563,7 +1565,7 @@ int hvm_vcpu_initialise(struct vcpu *v) (void(*)(unsigned long))hvm_assert_evtchn_irq, (unsigned long)v); - v->arch.hvm_vcpu.inject_trap.vector = -1; + v->arch.hvm_vcpu.inject_trap.vector = HVM_TRAP_VECTOR_UNSET; if ( is_pvh_domain(d) ) { diff --git a/xen/include/asm-x86/hvm/hvm.h b/xen/include/asm-x86/hvm/hvm.h index 8c95c08..bcacee3 100644 --- a/xen/include/asm-x86/hvm/hvm.h +++ b/xen/include/asm-x86/hvm/hvm.h @@ -77,6 +77,9 @@ enum hvm_intblk { #define HVM_HAP_SUPERPAGE_2MB 0x00000001 #define HVM_HAP_SUPERPAGE_1GB 0x00000002 +#define HVM_TRAP_VECTOR_UNSET (-1) +#define HVM_TRAP_VECTOR_UPDATING (-2) + /* * The hardware virtual machine (HVM) interface abstracts away from the * x86/x86_64 CPU virtualization assist specifics. Currently this interface