[v4,02/11] xen: move do_vcpu_op() to arch specific code

Commit Message

Juergen Gross March 10, 2022, 7:34 a.m. UTC

The entry point used for the vcpu_op hypercall on Arm is different
from the one on x86 today, as some of the common sub-ops are not
supported on Arm. The Arm specific handler filters out the not
supported sub-ops and then calls the common handler. This leads to the
weird call hierarchy:

  do_arm_vcpu_op()
    do_vcpu_op()
      arch_do_vcpu_op()

Clean this up by renaming do_vcpu_op() to common_vcpu_op() and
arch_do_vcpu_op() in each architecture to do_vcpu_op(). This way one
of above calls can be avoided without restricting any potential
future use of common sub-ops for Arm.

Signed-off-by: Juergen Gross <[e-mail]>
---
V4:
- don't remove HYPERCALL_ARM()
---
 xen/arch/arm/domain.c                | 15 ++++++++-------
 xen/arch/arm/include/asm/hypercall.h |  2 --
 xen/arch/arm/traps.c                 |  2 +-
 xen/arch/x86/domain.c                | 12 ++++++++----
 xen/arch/x86/include/asm/hypercall.h |  2 +-
 xen/arch/x86/x86_64/domain.c         | 16 ++++++++++++----
 xen/common/compat/domain.c           | 15 ++++++---------
 xen/common/domain.c                  | 12 ++++--------
 xen/include/xen/hypercall.h          |  2 +-
 9 files changed, 41 insertions(+), 37 deletions(-)

Comments

Julien Grall March 22, 2022, 8:13 p.m. UTC | #1

Hi Juergen,

On 10/03/2022 07:34, Juergen Gross wrote:
> diff --git a/xen/common/domain.c b/xen/common/domain.c
> index 351029f8b2..f9de1be43c 100644
> --- a/xen/common/domain.c
> +++ b/xen/common/domain.c
> @@ -1570,15 +1570,11 @@ int default_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
>       return rc;
>   }
>   
> -long cf_check do_vcpu_op(
> -    int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
> +long common_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
>   {
> -    struct domain *d = current->domain;
> -    struct vcpu *v;
>       long rc = 0;
> -
> -    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
> -        return -ENOENT;
> +    struct domain *d = current->domain;

It feels a bit strange to use current when you already have the vcpu in 
hand. So how about using v->domain?

The rest of the patch looks good to me.

Cheers,

Patch

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 8110c1df86..2f8eaab7b5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -1079,23 +1079,24 @@  void arch_dump_domain_info(struct domain *d)
 }
 
 
-long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
+long do_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
+    struct domain *d = current->domain;
+    struct vcpu *v;
+
+    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
+        return -ENOENT;
+
     switch ( cmd )
     {
         case VCPUOP_register_vcpu_info:
         case VCPUOP_register_runstate_memory_area:
-            return do_vcpu_op(cmd, vcpuid, arg);
+            return common_vcpu_op(cmd, v, arg);
         default:
             return -EINVAL;
     }
 }
 
-long arch_do_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
-{
-    return -ENOSYS;
-}
-
 void arch_dump_vcpu_info(struct vcpu *v)
 {
     gic_dump_info(v);
diff --git a/xen/arch/arm/include/asm/hypercall.h b/xen/arch/arm/include/asm/hypercall.h
index 39d2e7889d..fac4d60f17 100644
--- a/xen/arch/arm/include/asm/hypercall.h
+++ b/xen/arch/arm/include/asm/hypercall.h
@@ -4,8 +4,6 @@ 
 #include <public/domctl.h> /* for arch_do_domctl */
 int do_arm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg);
 
-long do_arm_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg);
-
 long subarch_do_domctl(struct xen_domctl *domctl, struct domain *d,
                        XEN_GUEST_HANDLE_PARAM(xen_domctl_t) u_domctl);
 
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index f8c3ef0ca2..deb07784d9 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1380,7 +1380,7 @@  static arm_hypercall_t arm_hypercall_table[] = {
 #endif
     HYPERCALL(multicall, 2),
     HYPERCALL(platform_op, 1),
-    HYPERCALL_ARM(vcpu_op, 3),
+    HYPERCALL(vcpu_op, 3),
     HYPERCALL(vm_assist, 2),
 #ifdef CONFIG_ARGO
     HYPERCALL(argo_op, 5),
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index a5048ed654..d566fc82b4 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1489,11 +1489,15 @@  int arch_vcpu_reset(struct vcpu *v)
     return 0;
 }
 
-long
-arch_do_vcpu_op(
-    int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
+long cf_check do_vcpu_op(int cmd, unsigned int vcpuid,
+                         XEN_GUEST_HANDLE_PARAM(void) arg)
 {
     long rc = 0;
+    struct domain *d = current->domain;
+    struct vcpu *v;
+
+    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
+        return -ENOENT;
 
     switch ( cmd )
     {
@@ -1545,7 +1549,7 @@  arch_do_vcpu_op(
     }
 
     default:
-        rc = -ENOSYS;
+        rc = common_vcpu_op(cmd, v, arg);
         break;
     }
 
diff --git a/xen/arch/x86/include/asm/hypercall.h b/xen/arch/x86/include/asm/hypercall.h
index 16d8418b57..610c37a0b8 100644
--- a/xen/arch/x86/include/asm/hypercall.h
+++ b/xen/arch/x86/include/asm/hypercall.h
@@ -145,7 +145,7 @@  compat_physdev_op(
     XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int
-arch_compat_vcpu_op(
+compat_common_vcpu_op(
     int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg);
 
 extern int cf_check compat_mmuext_op(
diff --git a/xen/arch/x86/x86_64/domain.c b/xen/arch/x86/x86_64/domain.c
index c46dccc25a..62fe51ee74 100644
--- a/xen/arch/x86/x86_64/domain.c
+++ b/xen/arch/x86/x86_64/domain.c
@@ -13,10 +13,14 @@  CHECK_vcpu_get_physid;
 #undef xen_vcpu_get_physid
 
 int
-arch_compat_vcpu_op(
-    int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
+compat_vcpu_op(int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    int rc = -ENOSYS;
+    int rc;
+    struct domain *d = current->domain;
+    struct vcpu *v;
+
+    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
+        return -ENOENT;
 
     switch ( cmd )
     {
@@ -55,7 +59,11 @@  arch_compat_vcpu_op(
     }
 
     case VCPUOP_get_physid:
-        rc = arch_do_vcpu_op(cmd, v, arg);
+        rc = do_vcpu_op(cmd, vcpuid, arg);
+        break;
+
+    default:
+        rc = compat_common_vcpu_op(cmd, v, arg);
         break;
     }
 
diff --git a/xen/common/compat/domain.c b/xen/common/compat/domain.c
index afae27eeba..1119534679 100644
--- a/xen/common/compat/domain.c
+++ b/xen/common/compat/domain.c
@@ -38,15 +38,12 @@  CHECK_vcpu_hvm_context;
 
 #endif
 
-int cf_check compat_vcpu_op(
-    int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
+int compat_common_vcpu_op(int cmd, struct vcpu *v,
+                          XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    struct domain *d = current->domain;
-    struct vcpu *v;
     int rc = 0;
-
-    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
-        return -ENOENT;
+    struct domain *d = current->domain;
+    unsigned int vcpuid = v->vcpu_id;
 
     switch ( cmd )
     {
@@ -103,7 +100,7 @@  int cf_check compat_vcpu_op(
     case VCPUOP_stop_singleshot_timer:
     case VCPUOP_register_vcpu_info:
     case VCPUOP_send_nmi:
-        rc = do_vcpu_op(cmd, vcpuid, arg);
+        rc = common_vcpu_op(cmd, v, arg);
         break;
 
     case VCPUOP_get_runstate_info:
@@ -134,7 +131,7 @@  int cf_check compat_vcpu_op(
     }
 
     default:
-        rc = arch_compat_vcpu_op(cmd, v, arg);
+        rc = -ENOSYS;
         break;
     }
 
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 351029f8b2..f9de1be43c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -1570,15 +1570,11 @@  int default_initialise_vcpu(struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
     return rc;
 }
 
-long cf_check do_vcpu_op(
-    int cmd, unsigned int vcpuid, XEN_GUEST_HANDLE_PARAM(void) arg)
+long common_vcpu_op(int cmd, struct vcpu *v, XEN_GUEST_HANDLE_PARAM(void) arg)
 {
-    struct domain *d = current->domain;
-    struct vcpu *v;
     long rc = 0;
-
-    if ( (v = domain_vcpu(d, vcpuid)) == NULL )
-        return -ENOENT;
+    struct domain *d = current->domain;
+    unsigned int vcpuid = v->vcpu_id;
 
     switch ( cmd )
     {
@@ -1750,7 +1746,7 @@  long cf_check do_vcpu_op(
     }
 
     default:
-        rc = arch_do_vcpu_op(cmd, v, arg);
+        rc = -ENOSYS;
         break;
     }
 
diff --git a/xen/include/xen/hypercall.h b/xen/include/xen/hypercall.h
index 4dfd64cf71..82f7f2caeb 100644
--- a/xen/include/xen/hypercall.h
+++ b/xen/include/xen/hypercall.h
@@ -110,7 +110,7 @@  do_vcpu_op(
 
 struct vcpu;
 extern long
-arch_do_vcpu_op(int cmd,
+common_vcpu_op(int cmd,
     struct vcpu *v,
     XEN_GUEST_HANDLE_PARAM(void) arg);