vm_event: Fix XEN_VM_EVENT_RESUME domctl

Commit Message

Petre Ovidiu PIRCALABU April 5, 2019, 9:33 a.m. UTC

Make XEN_VM_EVENT_RESUME return 0 in case of success, instead of
-EINVAL.
Remove vm_event_resume form vm_event.h header and set the function's
visibility to static as is used only in vm_event.c.
Move the vm_event_check_ring test inside vm_event_resume in order to
simplify the code.

Signed-off-by: Petre Pircalabu <[e-mail]>
---
 xen/common/vm_event.c      | 37 ++++++++++++-------------------------
 xen/include/xen/vm_event.h |  2 --
 2 files changed, 12 insertions(+), 27 deletions(-)

Comments

Razvan Cojocaru April 5, 2019, 9:56 a.m. UTC | #1

On 4/5/19 12:33 PM, Petre Pircalabu wrote:
> Make XEN_VM_EVENT_RESUME return 0 in case of success, instead of
> -EINVAL.
> Remove vm_event_resume form vm_event.h header and set the function's
> visibility to static as is used only in vm_event.c.
> Move the vm_event_check_ring test inside vm_event_resume in order to
> simplify the code.
> 
> Signed-off-by: Petre Pircalabu <[e-mail]>

Acked-by: Razvan Cojocaru <[e-mail]>

Jan Beulich April 5, 2019, 10:05 a.m. UTC | #2

>>> On 05.04.19 at 11:33, <[e-mail]> wrote:
> @@ -529,30 +534,21 @@ int __vm_event_claim_slot(struct domain *d, struct vm_event_domain *ved,
>  /* Registered with Xen-bound event channel for incoming notifications. */
>  static void mem_paging_notification(struct vcpu *v, unsigned int port)
>  {
> -    struct domain *domain = v->domain;
> -
> -    if ( likely(vm_event_check_ring(domain->vm_event_paging)) )
> -        vm_event_resume(domain, domain->vm_event_paging);
> +    (void)vm_event_resume(v->domain, v->domain->vm_event_paging);

Unless there's a good reason for them, could you please avoid
adding such casts?

Jan

Patch

diff --git a/xen/common/vm_event.c b/xen/common/vm_event.c
index 19c983c..746b040 100644
--- a/xen/common/vm_event.c
+++ b/xen/common/vm_event.c
@@ -366,7 +366,7 @@  int vm_event_get_response(struct domain *d, struct vm_event_domain *ved,
  * Note: responses are handled the same way regardless of which ring they
  * arrive on.
  */
-void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
+static int vm_event_resume(struct domain *d, struct vm_event_domain *ved)
 {
     vm_event_response_t rsp;
 
@@ -380,6 +380,9 @@  void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
      */
     ASSERT(d != current->domain);
 
+    if ( unlikely(!vm_event_check_ring(ved)) )
+         return -ENODEV;
+
     /* Pull all responses off the ring. */
     while ( vm_event_get_response(d, ved, &rsp) )
     {
@@ -443,6 +446,8 @@  void vm_event_resume(struct domain *d, struct vm_event_domain *ved)
                 vm_event_vcpu_unpause(v);
         }
     }
+
+    return 0;
 }
 
 void vm_event_cancel_slot(struct domain *d, struct vm_event_domain *ved)
@@ -529,30 +534,21 @@  int __vm_event_claim_slot(struct domain *d, struct vm_event_domain *ved,
 /* Registered with Xen-bound event channel for incoming notifications. */
 static void mem_paging_notification(struct vcpu *v, unsigned int port)
 {
-    struct domain *domain = v->domain;
-
-    if ( likely(vm_event_check_ring(domain->vm_event_paging)) )
-        vm_event_resume(domain, domain->vm_event_paging);
+    (void)vm_event_resume(v->domain, v->domain->vm_event_paging);
 }
 #endif
 
 /* Registered with Xen-bound event channel for incoming notifications. */
 static void monitor_notification(struct vcpu *v, unsigned int port)
 {
-    struct domain *domain = v->domain;
-
-    if ( likely(vm_event_check_ring(domain->vm_event_monitor)) )
-        vm_event_resume(domain, domain->vm_event_monitor);
+    (void)vm_event_resume(v->domain, v->domain->vm_event_monitor);
 }
 
 #ifdef CONFIG_HAS_MEM_SHARING
 /* Registered with Xen-bound event channel for incoming notifications. */
 static void mem_sharing_notification(struct vcpu *v, unsigned int port)
 {
-    struct domain *domain = v->domain;
-
-    if ( likely(vm_event_check_ring(domain->vm_event_share)) )
-        vm_event_resume(domain, domain->vm_event_share);
+    (void)vm_event_resume(v->domain, v->domain->vm_event_share);
 }
 #endif
 
@@ -676,10 +672,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
             break;
 
         case XEN_VM_EVENT_RESUME:
-            if ( vm_event_check_ring(d->vm_event_paging) )
-                vm_event_resume(d, d->vm_event_paging);
-            else
-                rc = -ENODEV;
+            rc = vm_event_resume(d, d->vm_event_paging);
             break;
 
         default:
@@ -717,10 +710,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
             break;
 
         case XEN_VM_EVENT_RESUME:
-            if ( vm_event_check_ring(d->vm_event_monitor) )
-                vm_event_resume(d, d->vm_event_monitor);
-            else
-                rc = -ENODEV;
+            rc = vm_event_resume(d, d->vm_event_monitor);
             break;
 
         default:
@@ -764,10 +754,7 @@  int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
             break;
 
         case XEN_VM_EVENT_RESUME:
-            if ( vm_event_check_ring(d->vm_event_share) )
-                vm_event_resume(d, d->vm_event_share);
-            else
-                rc = -ENODEV;
+            rc = vm_event_resume(d, d->vm_event_share);
             break;
 
         default:
diff --git a/xen/include/xen/vm_event.h b/xen/include/xen/vm_event.h
index 5302ee5..53af2d5 100644
--- a/xen/include/xen/vm_event.h
+++ b/xen/include/xen/vm_event.h
@@ -67,8 +67,6 @@  void vm_event_put_request(struct domain *d, struct vm_event_domain *ved,
 int vm_event_get_response(struct domain *d, struct vm_event_domain *ved,
                           vm_event_response_t *rsp);
 
-void vm_event_resume(struct domain *d, struct vm_event_domain *ved);
-
 int vm_event_domctl(struct domain *d, struct xen_domctl_vm_event_op *vec,
                     XEN_GUEST_HANDLE_PARAM(void) u_domctl);