diff mbox

[v6,4/5] vm_event: clear up return value of vm_event_monitor_traps

Message ID 1466701647-21733-4-git-send-email-tamas@tklengyel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tamas K Lengyel June 23, 2016, 5:07 p.m. UTC 
The return value has not been clearly defined, with the function
never returning 0 which seemingly indicated a condition where the
guest should crash.

In this patch we define -rc as error condition where a subscriber is
present but an error prevented the notification from being sent;
0 where there is no subscriber or the notification was sent and the vCPU
is not paused (i.e. safe to continue execution as normal); and 1 where the
notification was sent with the vCPU paused and we are waiting for a
response.

Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>
---
Cc: Jun Nakajima <jun.nakajima@intel.com>
Cc: Kevin Tian <kevin.tian@intel.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v6: Remove unnecessary else clause in vmx
---
 xen/arch/x86/hvm/monitor.c | 4 ++--
 xen/arch/x86/hvm/vmx/vmx.c | 6 +++---
 xen/common/vm_event.c      | 5 +++--
 3 files changed, 8 insertions(+), 7 deletions(-)

Comments

Tian, Kevin June 24, 2016, 10:59 a.m. UTC | #1 
> From: Tamas K Lengyel [mailto:tamas@tklengyel.com]
> Sent: Friday, June 24, 2016 1:07 AM
> 
> The return value has not been clearly defined, with the function
> never returning 0 which seemingly indicated a condition where the
> guest should crash.
> 
> In this patch we define -rc as error condition where a subscriber is
> present but an error prevented the notification from being sent;
> 0 where there is no subscriber or the notification was sent and the vCPU
> is not paused (i.e. safe to continue execution as normal); and 1 where the
> notification was sent with the vCPU paused and we are waiting for a
> response.
> 
> Signed-off-by: Tamas K Lengyel <tamas@tklengyel.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> Acked-by: Razvan Cojocaru <rcojocaru@bitdefender.com>

Sorry I should ack this new version:

Acked-by: Kevin Tian <kevin.tian@intel.com>
diff mbox

Patch

diff --git a/xen/arch/x86/hvm/monitor.c b/xen/arch/x86/hvm/monitor.c
index f0ab33a..472926c 100644
--- a/xen/arch/x86/hvm/monitor.c
+++ b/xen/arch/x86/hvm/monitor.c
@@ -48,8 +48,8 @@  bool_t hvm_monitor_cr(unsigned int index, unsigned long value, unsigned long old
             .u.write_ctrlreg.old_value = old
         };
 
-        vm_event_monitor_traps(curr, sync, &req);
-        return 1;
+        if ( vm_event_monitor_traps(curr, sync, &req) >= 0 )
+            return 1;
     }
 
     return 0;
diff --git a/xen/arch/x86/hvm/vmx/vmx.c b/xen/arch/x86/hvm/vmx/vmx.c
index a56926c..03fcba7 100644
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3388,11 +3388,11 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
                 break;
             }
             else {
-                int handled =
+                int rc =
                       hvm_monitor_breakpoint(regs->eip,
                                              HVM_MONITOR_SOFTWARE_BREAKPOINT);
 
-                if ( handled < 0 ) 
+                if ( !rc )
                 {
                     struct hvm_trap trap = {
                         .vector = TRAP_int3,
@@ -3406,7 +3406,7 @@  void vmx_vmexit_handler(struct cpu_user_regs *regs)
                     hvm_inject_trap(&trap);
                     break;
                 }
-                else if ( handled )
+                if ( rc > 0 )
                     break;
             }
 
diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index ca1eced..b303180 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -806,7 +806,7 @@  int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
          * If there was no ring to handle the event, then
          * simply continue executing normally.
          */
-        return 1;
+        return 0;
     default:
         return rc;
     };
@@ -815,6 +815,7 @@  int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
     {
         req->flags |= VM_EVENT_FLAG_VCPU_PAUSED;
         vm_event_vcpu_pause(v);
+        rc = 1;
     }
 
     if ( altp2m_active(d) )
@@ -826,7 +827,7 @@  int vm_event_monitor_traps(struct vcpu *v, uint8_t sync,
     vm_event_fill_regs(req);
     vm_event_put_request(d, &d->vm_event->monitor, req);
 
-    return 1;
+    return rc;
 }
 
 /*