[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 1/9] xen: move do_vcpu_op() to arch specific code

To: xen-devel@xxxxxxxxxxxxxxxxxxxx, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
From: Juergen Gross <jgross@xxxxxxxx>
Date: Tue, 7 Jun 2022 07:00:15 +0200
Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Bertrand Marquis <bertrand.marquis@xxxxxxx>, Volodymyr Babchuk <Volodymyr_Babchuk@xxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Jan Beulich <jbeulich@xxxxxxxx>, Wei Liu <wl@xxxxxxx>, Julien Grall <jgrall@xxxxxxxxxx>
Delivery-date: Tue, 07 Jun 2022 05:00:36 +0000
List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 24.03.22 15:01, Juergen Gross wrote:

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 <jgross@xxxxxxxx>
Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>


There is still an Ack missing for x86 side. Jan already said he isn't
happy to give one, but won't Nack it. Roger, Andrew, any comments for
this patch? It is blocking further cleanup patches to go in (patches
2 and 4 of this series).


Juergen

---
V4:
- don't remove HYPERCALL_ARM()
V4.1:
- add missing cf_check (Andrew Cooper)
V5:
- use v instead of current (Julien Grall)
---
  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         | 18 +++++++++++++-----
  xen/common/compat/domain.c           | 15 ++++++---------
  xen/common/domain.c                  | 12 ++++--------
  xen/include/xen/hypercall.h          |  2 +-
  9 files changed, 42 insertions(+), 38 deletions(-)

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 43f30747cf..e906bb4a89 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 61bf897147..d6daa7e4cb 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..9c559aa3ea 100644
--- a/xen/arch/x86/x86_64/domain.c
+++ b/xen/arch/x86/x86_64/domain.c
@@ -12,11 +12,15 @@
  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)
+int cf_check
+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..70747c02e6 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 = v->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 a1b6575976..81aae7a662 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);

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Prev by Date: memory overcomittment with sr-iov device assighment
Next by Date: Re: [PATCH v6 0/9] xen: drop hypercall function tables
Previous by thread: memory overcomittment with sr-iov device assighment
Next by thread: Re: [PATCH v6 1/9] xen: move do_vcpu_op() to arch specific code
Index(es):
- Date
- Thread

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.