Re: [Xen-devel] [PATCH 11/11] x86/HVM: serialize trap injecting producer and consumer

[Andrew Cooper]

On 20/06/16 13:58, Jan Beulich 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 prodcing and consuming sides. Fix this by leveraging the

producing.

> vector field as synchronization variable.
>
> Signed-off-by: Jan Beulich 

Reviewed-by: Andrew Cooper 

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH 11/11] x86/HVM: serialize trap injecting producer and consumer

[Jan Beulich]

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 prodcing and consuming sides. Fix this by leveraging the
vector field as synchronization variable.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -106,14 +106,16 @@ static int inject_trap(struct domain *d,
 if ( op->vcpuid >= d->max_vcpus || (v = d->vcpu[op->vcpuid]) == NULL )
 return -ENOENT;
 
-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.vector = op->vector;
 v->arch.hvm_vcpu.inject_trap.type   = op->type;
 v->arch.hvm_vcpu.inject_trap.error_code = op->error_code;
 v->arch.hvm_vcpu.inject_trap.insn_len   = op->insn_len;
 v->arch.hvm_vcpu.inject_trap.cr2= op->cr2;
+smp_wmb();
+v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 
 return 0;
 }
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -510,10 +510,11 @@ 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();
 hvm_inject_trap(&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;
 }
 }
 
@@ -1508,7 +1509,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) )
 {
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -78,6 +78,8 @@ enum hvm_intblk {
 #define HVM_HAP_SUPERPAGE_1GB   0x0002
 
 struct hvm_trap {
+#define HVM_TRAP_VECTOR_UNSET(-1)
+#define HVM_TRAP_VECTOR_UPDATING (-2)
 int16_t   vector;
 uint8_t   type; /* X86_EVENTTYPE_* */
 uint8_t   insn_len; /* Instruction length */



x86/HVM: serialize trap injecting producer and consumer

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 prodcing and consuming sides. Fix this by leveraging the
vector field as synchronization variable.

Signed-off-by: Jan Beulich 

--- a/xen/arch/x86/hvm/control.c
+++ b/xen/arch/x86/hvm/control.c
@@ -106,14 +106,16 @@ static int inject_trap(struct domain *d,
 if ( op->vcpuid >= d->max_vcpus || (v = d->vcpu[op->vcpuid]) == NULL )
 return -ENOENT;
 
-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.vector = op->vector;
 v->arch.hvm_vcpu.inject_trap.type   = op->type;
 v->arch.hvm_vcpu.inject_trap.error_code = op->error_code;
 v->arch.hvm_vcpu.inject_trap.insn_len   = op->insn_len;
 v->arch.hvm_vcpu.inject_trap.cr2= op->cr2;
+smp_wmb();
+v->arch.hvm_vcpu.inject_trap.vector = op->vector;
 
 return 0;
 }
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -510,10 +510,11 @@ 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();
 hvm_inject_trap(&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;
 }
 }
 
@@ -1508,7 +1509,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) )
 {
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -78,6 +78,8 @@ enum hvm_intblk {
 #define HVM_HAP_SUPERPAGE_1GB   0x0002
 
 struct hvm_trap {
+#define HVM_TRAP_VECTOR_UNSET(-1)
+#define HVM_TRAP_VECTOR_UPDATING (-2)
 int16_t   vector;
 uint8_t   type; /* X86_EVENTTYPE_* */
 uint8_t   insn_len; /* Instruction length */
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel