[PATCH RFC] x86/xen: Return error for xc_hvm_inject_trap() with pending events

[PATCH RFC] x86/xen: Return error for xc_hvm_inject_trap() with pending events

[Razvan Cojocaru]

xc_hvm_inject_trap() sets v->arch.hvm_vcpu.inject_trap.vector,
which is then checked in hvm_do_resume(), and if != -1, a trap
is injected, regardless of whether vmx_idtv_reinject() has written
VM_ENTRY_INTR_INFO directly. If that's the case, the toolstack
injected interrupt will overwrite the reinjected one, which will
get lost forever. This patch returns -EBUSY not only if
v->arch.hvm_vcpu.inject_trap.vector != -1, but also if
hvm_event_pending(v). hvm_event_pending() has also been modified
to be able to run on a VCPU which is not current.

Signed-off-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
 xen/arch/x86/hvm/hvm.c     | 2 +-
 xen/arch/x86/hvm/vmx/vmx.c | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 704fd64..cf01ae4 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -5938,7 +5938,7 @@ long do_hvm_op(unsigned long op, XEN_GUEST_HANDLE_PARAM(void) arg)
         if ( tr.vcpuid >= d->max_vcpus || (v = d->vcpu[tr.vcpuid]) == NULL )
             goto injtrap_fail;
         
-        if ( v->arch.hvm_vcpu.inject_trap.vector != -1 )
+        if ( v->arch.hvm_vcpu.inject_trap.vector != -1 || hvm_event_pending(v) )
             rc = -EBUSY;
         else 
         {
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index 9a8f694..f50a593 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -1790,8 +1790,9 @@ static int vmx_event_pending(struct vcpu *v)
 {
     unsigned long intr_info;
 
-    ASSERT(v == current);
+    vmx_vmcs_enter(v);
     __vmread(VM_ENTRY_INTR_INFO, &intr_info);
+    vmx_vmcs_exit(v);
 
     return intr_info & INTR_INFO_VALID_MASK;
 }
-- 
1.9.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] x86/xen: Return error for xc_hvm_inject_trap() with pending events

[Jan Beulich]

>>> On 08.11.16 at 13:16, <rcojocaru@bitdefender.com> wrote:
> xc_hvm_inject_trap() sets v->arch.hvm_vcpu.inject_trap.vector,
> which is then checked in hvm_do_resume(), and if != -1, a trap
> is injected, regardless of whether vmx_idtv_reinject() has written
> VM_ENTRY_INTR_INFO directly. If that's the case, the toolstack
> injected interrupt will overwrite the reinjected one, which will
> get lost forever. This patch returns -EBUSY not only if
> v->arch.hvm_vcpu.inject_trap.vector != -1, but also if
> hvm_event_pending(v).

Considering the earlier discussion I don't understand why this was
put together and submitted: It is my understanding that it won't
cover all possible cases of actual injection not succeeding, and I
can't see what good a partial solution will do for you or anyone
else.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [PATCH RFC] x86/xen: Return error for xc_hvm_inject_trap() with pending events

[Razvan Cojocaru]

On 11/08/2016 03:10 PM, Jan Beulich wrote:
>>>> On 08.11.16 at 13:16, <rcojocaru@bitdefender.com> wrote:
>> xc_hvm_inject_trap() sets v->arch.hvm_vcpu.inject_trap.vector,
>> which is then checked in hvm_do_resume(), and if != -1, a trap
>> is injected, regardless of whether vmx_idtv_reinject() has written
>> VM_ENTRY_INTR_INFO directly. If that's the case, the toolstack
>> injected interrupt will overwrite the reinjected one, which will
>> get lost forever. This patch returns -EBUSY not only if
>> v->arch.hvm_vcpu.inject_trap.vector != -1, but also if
>> hvm_event_pending(v).
> 
> Considering the earlier discussion I don't understand why this was
> put together and submitted: It is my understanding that it won't
> cover all possible cases of actual injection not succeeding, and I
> can't see what good a partial solution will do for you or anyone
> else.

Well, the previous discussion also moved quite a bit through proposed
solutions, and this is the smallest patch that helps.

We're always injecting traps when the VCPU is paused (i.e. when handling
a sync vm_event), and at that time I believe that xc_hvm_inject_trap()
succeeding with this patch would guarantee that the interrupt can be
delivered, and it failing immediately will notify us that injection is
not currently possible.

So for the flow which prompted the discussion, this is a solution. Also,
the patch is RFC - I thought seeing a concrete patch would help, rather
than going on with more abstract concerns.

I'll go back to the vm_event patch then.


Thanks,
Razvan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel