All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Common hypercall handing improvements
@ 2017-02-15 19:41 Andrew Cooper
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common Andrew Cooper
                   ` (6 more replies)
  0 siblings, 7 replies; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper

This original purpose of this work was towards the RFC patch at the end of the
series, but there turned out to be a lot of cleanup opportunity in common and
ARM code as well.

Andrew Cooper (7):
  x86/hypercall: Make the HVM hcall_preempted boolean common
  arm/hypercall: Use the common hcall_preempted boolean
  xen/multicall: Use the common hcall_preempted boolean
  x86/hypercall: Make the HVM hcall_64bit boolean common
  x86/hypercall: Split out PV hypercall infrastructure
  x86/hypercall: Move hypercall continuation logic
  [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM

 xen/arch/arm/domain.c              |  19 +--
 xen/arch/arm/traps.c               |  16 +-
 xen/arch/x86/Kconfig               |  68 ++++++--
 xen/arch/x86/Makefile              |   3 +-
 xen/arch/x86/domain.c              | 198 ----------------------
 xen/arch/x86/hvm/hvm.c             |   9 +-
 xen/arch/x86/hvm/hypercall.c       |  19 +--
 xen/arch/x86/hypercall.c           | 326 +++++++++++++++++--------------------
 xen/arch/x86/pv/Makefile           |   1 +
 xen/arch/x86/{ => pv}/hypercall.c  |  78 ++-------
 xen/common/multicall.c             |   4 +-
 xen/include/asm-x86/hvm/vcpu.h     |   3 -
 xen/include/xen/multicall.h        |   2 -
 xen/include/xen/sched.h            |   6 +
 xen/tools/kconfig/allrandom.config |   4 +
 15 files changed, 252 insertions(+), 504 deletions(-)
 create mode 100644 xen/arch/x86/pv/Makefile
 copy xen/arch/x86/{ => pv}/hypercall.c (82%)

-- 
2.1.4


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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 10:44   ` [PATCH 1/7] " Jan Beulich
  2017-02-15 19:41 ` [PATCH] arm/hypercall: Use the common hcall_preempted boolean Andrew Cooper
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

HVM guests currently make use of arch.hvm_vcpu.hcall_preempted to track
hypercall preemption in struct vcpu.  Move this boolean to being common at the
top level of struct vcpu, which will allow it to be reused elsewhere.

Alter the PV preemption logic to use this boolean.  This simplifies the code
by removing guest-type-specific knowledge, and removes the risk of accidently
skipping backwards or forwards multiple times and corrupting %rip.

In pv_hypercall() the old_rip bodge can be removed, and parameter clobbering
can happen based on a more obvious condition.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/domain.c          | 16 ++--------------
 xen/arch/x86/hvm/hypercall.c   |  8 ++++----
 xen/arch/x86/hypercall.c       | 17 ++++++++++++-----
 xen/include/asm-x86/hvm/vcpu.h |  1 -
 xen/include/xen/sched.h        |  2 ++
 5 files changed, 20 insertions(+), 24 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 71c0e3c..b199c70 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2198,20 +2198,12 @@ void sync_vcpu_execstate(struct vcpu *v)
 
 void hypercall_cancel_continuation(void)
 {
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
     struct mc_state *mcs = &current->mc_state;
 
     if ( mcs->flags & MCSF_in_multicall )
-    {
         __clear_bit(_MCSF_call_preempted, &mcs->flags);
-    }
     else
-    {
-        if ( is_pv_vcpu(current) )
-            regs->rip += 2; /* skip re-execute 'syscall' / 'int $xx' */
-        else
-            current->arch.hvm_vcpu.hcall_preempted = 0;
-    }
+        current->hcall_preempted = false;
 }
 
 unsigned long hypercall_create_continuation(
@@ -2239,11 +2231,7 @@ unsigned long hypercall_create_continuation(
 
         regs->rax = op;
 
-        /* Ensure the hypercall trap instruction is re-executed. */
-        if ( is_pv_vcpu(curr) )
-            regs->rip -= 2;  /* re-execute 'syscall' / 'int $xx' */
-        else
-            curr->arch.hvm_vcpu.hcall_preempted = 1;
+        curr->hcall_preempted = true;
 
         if ( is_pv_vcpu(curr) ?
              !is_pv_32bit_vcpu(curr) :
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index 5dbd54a..fe7802b 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -176,7 +176,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
         return HVM_HCALL_completed;
     }
 
-    curr->arch.hvm_vcpu.hcall_preempted = 0;
+    curr->hcall_preempted = false;
 
     if ( mode == 8 )
     {
@@ -210,7 +210,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
         curr->arch.hvm_vcpu.hcall_64bit = 0;
 
 #ifndef NDEBUG
-        if ( !curr->arch.hvm_vcpu.hcall_preempted )
+        if ( !curr->hcall_preempted )
         {
             /* Deliberately corrupt parameter regs used by this hypercall. */
             switch ( hypercall_args_table[eax].native )
@@ -254,7 +254,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
                                                     ebp);
 
 #ifndef NDEBUG
-        if ( !curr->arch.hvm_vcpu.hcall_preempted )
+        if ( !curr->hcall_preempted )
         {
             /* Deliberately corrupt parameter regs used by this hypercall. */
             switch ( hypercall_args_table[eax].compat )
@@ -272,7 +272,7 @@ int hvm_hypercall(struct cpu_user_regs *regs)
 
     HVM_DBG_LOG(DBG_LEVEL_HCALL, "hcall%lu -> %lx", eax, regs->rax);
 
-    if ( curr->arch.hvm_vcpu.hcall_preempted )
+    if ( curr->hcall_preempted )
         return HVM_HCALL_preempted;
 
     if ( unlikely(currd->arch.hvm_domain.qemu_mapcache_invalidate) &&
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 8dd19de..945afa0 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -141,9 +141,6 @@ static const hypercall_table_t pv_hypercall_table[] = {
 void pv_hypercall(struct cpu_user_regs *regs)
 {
     struct vcpu *curr = current;
-#ifndef NDEBUG
-    unsigned long old_rip = regs->rip;
-#endif
     unsigned long eax;
 
     ASSERT(guest_kernel_mode(curr, regs));
@@ -160,6 +157,8 @@ void pv_hypercall(struct cpu_user_regs *regs)
         return;
     }
 
+    curr->hcall_preempted = false;
+
     if ( !is_pv_32bit_vcpu(curr) )
     {
         unsigned long rdi = regs->rdi;
@@ -191,7 +190,7 @@ void pv_hypercall(struct cpu_user_regs *regs)
         regs->rax = pv_hypercall_table[eax].native(rdi, rsi, rdx, r10, r8, r9);
 
 #ifndef NDEBUG
-        if ( regs->rip == old_rip )
+        if ( !curr->hcall_preempted )
         {
             /* Deliberately corrupt parameter regs used by this hypercall. */
             switch ( hypercall_args_table[eax].native )
@@ -238,7 +237,7 @@ void pv_hypercall(struct cpu_user_regs *regs)
         regs->_eax = pv_hypercall_table[eax].compat(ebx, ecx, edx, esi, edi, ebp);
 
 #ifndef NDEBUG
-        if ( regs->rip == old_rip )
+        if ( !curr->hcall_preempted )
         {
             /* Deliberately corrupt parameter regs used by this hypercall. */
             switch ( hypercall_args_table[eax].compat )
@@ -254,6 +253,14 @@ void pv_hypercall(struct cpu_user_regs *regs)
 #endif
     }
 
+    /*
+     * PV guests use SYSCALL or INT $0x82 to make a hypercall, both of which
+     * have trap semantics.  If the hypercall has been preempted, rewind the
+     * instruction pointer to reexecute the instruction.
+     */
+    if ( curr->hcall_preempted )
+        regs->rip -= 2;
+
     perfc_incr(hypercalls);
 }
 
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index e5eeb5f..6d5553d 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -166,7 +166,6 @@ struct hvm_vcpu {
     bool                debug_state_latch;
     bool                single_step;
 
-    bool                hcall_preempted;
     bool                hcall_64bit;
 
     struct hvm_vcpu_asid n1asid;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 32893de..5b62238 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -202,6 +202,8 @@ struct vcpu
     bool             paused_for_shutdown;
     /* VCPU need affinity restored */
     bool             affinity_broken;
+    /* A hypercall has been preempted. */
+    bool             hcall_preempted;
 
 
     /*
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] arm/hypercall: Use the common hcall_preempted boolean
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 12:04   ` Julien Grall
  2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini

With hcall_preempted having just been made common, ARM can use use it to
simplify its hypercall handling.

This simplifies the continuation logic and removes the risk of accidentally
skipping multiple instructions.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>

NB: Only compile tested, not functionally tested.  However, it is basically an
identical transformation as done to x86 PV guests in the previous patch, which
I have tested thoroughly.
---
 xen/arch/arm/domain.c | 10 ++--------
 xen/arch/arm/traps.c  | 16 ++++++++--------
 2 files changed, 10 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 7e43691..fb1d8a5 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -349,17 +349,12 @@ void sync_vcpu_execstate(struct vcpu *v)
 
 void hypercall_cancel_continuation(void)
 {
-    struct cpu_user_regs *regs = guest_cpu_user_regs();
     struct mc_state *mcs = &current->mc_state;
 
     if ( mcs->flags & MCSF_in_multicall )
-    {
         __clear_bit(_MCSF_call_preempted, &mcs->flags);
-    }
     else
-    {
-        regs->pc += 4; /* undo re-execute 'hvc #XEN_HYPERCALL_TAG' */
-    }
+        current->hcall_preempted = false;
 }
 
 unsigned long hypercall_create_continuation(
@@ -391,8 +386,7 @@ unsigned long hypercall_create_continuation(
     {
         regs = guest_cpu_user_regs();
 
-        /* Ensure the hypercall trap instruction is re-executed. */
-        regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
+        current->hcall_preempted = true;
 
 #ifdef CONFIG_ARM_64
         if ( !is_32bit_domain(current->domain) )
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 92b1d80..7c99dc2 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1454,9 +1454,6 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
                               unsigned long iss)
 {
     arm_hypercall_fn_t call = NULL;
-#ifndef NDEBUG
-    register_t orig_pc = regs->pc;
-#endif
 
     BUILD_BUG_ON(NR_hypercalls < ARRAY_SIZE(arm_hypercall_table) );
 
@@ -1470,6 +1467,8 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
         return;
     }
 
+    current->hcall_preempted = false;
+
     perfc_incra(hypercalls, *nr);
     call = arm_hypercall_table[*nr].fn;
     if ( call == NULL )
@@ -1481,12 +1480,9 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
     HYPERCALL_RESULT_REG(regs) = call(HYPERCALL_ARGS(regs));
 
 #ifndef NDEBUG
-    /*
-     * Clobber argument registers only if pc is unchanged, otherwise
-     * this is a hypercall continuation.
-     */
-    if ( orig_pc == regs->pc )
+    if ( !current->hcall_preempted )
     {
+        /* Deliberately corrupt parameter regs used by this hypercall. */
         switch ( arm_hypercall_table[*nr].nr_args ) {
         case 5: HYPERCALL_ARG5(regs) = 0xDEADBEEF;
         case 4: HYPERCALL_ARG4(regs) = 0xDEADBEEF;
@@ -1499,6 +1495,10 @@ static void do_trap_hypercall(struct cpu_user_regs *regs, register_t *nr,
         *nr = 0xDEADBEEF;
     }
 #endif
+
+    /* Ensure the hypercall trap instruction is re-executed. */
+    if ( current->hcall_preempted )
+        regs->pc -= 4;  /* re-execute 'hvc #XEN_HYPERCALL_TAG' */
 }
 
 static bool check_multicall_32bit_clean(struct multicall_entry *multi)
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] xen/multicall: Use the common hcall_preempted boolean
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common Andrew Cooper
  2017-02-15 19:41 ` [PATCH] arm/hypercall: Use the common hcall_preempted boolean Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 10:37   ` Jan Beulich
                     ` (2 more replies)
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_64bit boolean common Andrew Cooper
                   ` (3 subsequent siblings)
  6 siblings, 3 replies; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Julien Grall, Stefano Stabellini, Jan Beulich

The now-common hcall_preempted boolean is perfectly usable for multicalls.
Remove the multicall-specific preemption mechanism.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/domain.c       | 13 +++----------
 xen/arch/x86/domain.c       | 19 ++++++-------------
 xen/common/multicall.c      |  4 +++-
 xen/include/xen/multicall.h |  2 --
 4 files changed, 12 insertions(+), 26 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index fb1d8a5..39b6eb8 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -349,12 +349,7 @@ void sync_vcpu_execstate(struct vcpu *v)
 
 void hypercall_cancel_continuation(void)
 {
-    struct mc_state *mcs = &current->mc_state;
-
-    if ( mcs->flags & MCSF_in_multicall )
-        __clear_bit(_MCSF_call_preempted, &mcs->flags);
-    else
-        current->hcall_preempted = false;
+    current->hcall_preempted = false;
 }
 
 unsigned long hypercall_create_continuation(
@@ -370,12 +365,12 @@ unsigned long hypercall_create_continuation(
     /* All hypercalls take at least one argument */
     BUG_ON( !p || *p == '\0' );
 
+    current->hcall_preempted = true;
+
     va_start(args, format);
 
     if ( mcs->flags & MCSF_in_multicall )
     {
-        __set_bit(_MCSF_call_preempted, &mcs->flags);
-
         for ( i = 0; *p != '\0'; i++ )
             mcs->call.args[i] = next_arg(p, args);
 
@@ -386,8 +381,6 @@ unsigned long hypercall_create_continuation(
     {
         regs = guest_cpu_user_regs();
 
-        current->hcall_preempted = true;
-
 #ifdef CONFIG_ARM_64
         if ( !is_32bit_domain(current->domain) )
         {
diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index b199c70..08c5813 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2198,41 +2198,34 @@ void sync_vcpu_execstate(struct vcpu *v)
 
 void hypercall_cancel_continuation(void)
 {
-    struct mc_state *mcs = &current->mc_state;
-
-    if ( mcs->flags & MCSF_in_multicall )
-        __clear_bit(_MCSF_call_preempted, &mcs->flags);
-    else
-        current->hcall_preempted = false;
+    current->hcall_preempted = false;
 }
 
 unsigned long hypercall_create_continuation(
     unsigned int op, const char *format, ...)
 {
-    struct mc_state *mcs = &current->mc_state;
+    struct vcpu *curr = current;
+    struct mc_state *mcs = &curr->mc_state;
     const char *p = format;
     unsigned long arg;
     unsigned int i;
     va_list args;
 
+    curr->hcall_preempted = true;
+
     va_start(args, format);
 
     if ( mcs->flags & MCSF_in_multicall )
     {
-        __set_bit(_MCSF_call_preempted, &mcs->flags);
-
         for ( i = 0; *p != '\0'; i++ )
             mcs->call.args[i] = next_arg(p, args);
     }
     else
     {
         struct cpu_user_regs *regs = guest_cpu_user_regs();
-        struct vcpu *curr = current;
 
         regs->rax = op;
 
-        curr->hcall_preempted = true;
-
         if ( is_pv_vcpu(curr) ?
              !is_pv_32bit_vcpu(curr) :
              curr->arch.hvm_vcpu.hcall_64bit )
@@ -2293,7 +2286,7 @@ int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
 
     if ( mcs->flags & MCSF_in_multicall )
     {
-        if ( !(mcs->flags & MCSF_call_preempted) )
+        if ( !current->hcall_preempted )
         {
             va_end(args);
             return 0;
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index 524c9bf..02f57cb 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -79,7 +79,7 @@ do_multicall(
 
         if ( unlikely(__copy_field_to_guest(call_list, &mcs->call, result)) )
             rc = -EFAULT;
-        else if ( mcs->flags & MCSF_call_preempted )
+        else if ( current->hcall_preempted )
         {
             /* Translate sub-call continuation to guest layout */
             xlat_multicall_entry(mcs);
@@ -87,6 +87,8 @@ do_multicall(
             /* Copy the sub-call continuation. */
             if ( likely(!__copy_to_guest(call_list, &mcs->call, 1)) )
                 goto preempted;
+            else
+                hypercall_cancel_continuation();
             rc = -EFAULT;
         }
         else
diff --git a/xen/include/xen/multicall.h b/xen/include/xen/multicall.h
index fff15eb..e8d7905 100644
--- a/xen/include/xen/multicall.h
+++ b/xen/include/xen/multicall.h
@@ -11,9 +11,7 @@
 #endif
 
 #define _MCSF_in_multicall   0
-#define _MCSF_call_preempted 1
 #define MCSF_in_multicall    (1<<_MCSF_in_multicall)
-#define MCSF_call_preempted  (1<<_MCSF_call_preempted)
 struct mc_state {
     unsigned long flags;
     union {
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] x86/hypercall: Make the HVM hcall_64bit boolean common
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
                   ` (2 preceding siblings ...)
  2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 11:07   ` [PATCH 4/7] " Jan Beulich
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Split out PV hypercall infrastructure Andrew Cooper
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

HVM guests currently make use of arch.hvm_vcpu.hcall_64bit to track the ABI of
the hypercall in use.

The rest of Xen deals in terms of the comat ABI or not, so rename the boolean
and make it common, guared by CONFIG_COMPAT to avoid bloat if a compat ABI is
not wanted/needed.

Set hcall_compat uniformly for PV guests as well as HVM guests.  This removes
the remaining piece of guest-type-specific knowledge from
hypercall_create_continuation(), allowing it to operate only in terms of the
hypercall ABI in use.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/domain.c          |  4 +---
 xen/arch/x86/hvm/hvm.c         |  9 +++------
 xen/arch/x86/hvm/hypercall.c   | 11 +++++------
 xen/arch/x86/hypercall.c       |  2 ++
 xen/include/asm-x86/hvm/vcpu.h |  2 --
 xen/include/xen/sched.h        |  4 ++++
 6 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 08c5813..3209eb3 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2226,9 +2226,7 @@ unsigned long hypercall_create_continuation(
 
         regs->rax = op;
 
-        if ( is_pv_vcpu(curr) ?
-             !is_pv_32bit_vcpu(curr) :
-             curr->arch.hvm_vcpu.hcall_64bit )
+        if ( !curr->hcall_compat )
         {
             for ( i = 0; *p != '\0'; i++ )
             {
diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
index 266f708..4d29e3c 100644
--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -3254,8 +3254,7 @@ unsigned long copy_to_user_hvm(void *to, const void *from, unsigned int len)
 {
     int rc;
 
-    if ( !current->arch.hvm_vcpu.hcall_64bit &&
-         is_compat_arg_xlat_range(to, len) )
+    if ( current->hcall_compat && is_compat_arg_xlat_range(to, len) )
     {
         memcpy(to, from, len);
         return 0;
@@ -3269,8 +3268,7 @@ unsigned long clear_user_hvm(void *to, unsigned int len)
 {
     int rc;
 
-    if ( !current->arch.hvm_vcpu.hcall_64bit &&
-         is_compat_arg_xlat_range(to, len) )
+    if ( current->hcall_compat && is_compat_arg_xlat_range(to, len) )
     {
         memset(to, 0x00, len);
         return 0;
@@ -3284,8 +3282,7 @@ unsigned long copy_from_user_hvm(void *to, const void *from, unsigned len)
 {
     int rc;
 
-    if ( !current->arch.hvm_vcpu.hcall_64bit &&
-         is_compat_arg_xlat_range(from, len) )
+    if ( current->hcall_compat && is_compat_arg_xlat_range(from, len) )
     {
         memcpy(to, from, len);
         return 0;
diff --git a/xen/arch/x86/hvm/hypercall.c b/xen/arch/x86/hvm/hypercall.c
index fe7802b..0f7c310 100644
--- a/xen/arch/x86/hvm/hypercall.c
+++ b/xen/arch/x86/hvm/hypercall.c
@@ -35,7 +35,7 @@ static long hvm_memory_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         return -ENOSYS;
     }
 
-    if ( curr->arch.hvm_vcpu.hcall_64bit )
+    if ( !curr->hcall_compat )
         rc = do_memory_op(cmd, arg);
     else
         rc = compat_memory_op(cmd, arg);
@@ -65,7 +65,7 @@ static long hvm_grant_table_op(
         return -ENOSYS;
     }
 
-    if ( current->arch.hvm_vcpu.hcall_64bit )
+    if ( !current->hcall_compat )
         return do_grant_table_op(cmd, uop, count);
     else
         return compat_grant_table_op(cmd, uop, count);
@@ -89,7 +89,7 @@ static long hvm_physdev_op(int cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
         break;
     }
 
-    if ( curr->arch.hvm_vcpu.hcall_64bit )
+    if ( !curr->hcall_compat )
         return do_physdev_op(cmd, arg);
     else
         return compat_physdev_op(cmd, arg);
@@ -203,12 +203,9 @@ int hvm_hypercall(struct cpu_user_regs *regs)
         }
 #endif
 
-        curr->arch.hvm_vcpu.hcall_64bit = 1;
         regs->rax = hvm_hypercall_table[eax].native(rdi, rsi, rdx, r10, r8,
                                                     r9);
 
-        curr->arch.hvm_vcpu.hcall_64bit = 0;
-
 #ifndef NDEBUG
         if ( !curr->hcall_preempted )
         {
@@ -250,8 +247,10 @@ int hvm_hypercall(struct cpu_user_regs *regs)
         }
 #endif
 
+        curr->hcall_compat = true;
         regs->rax = hvm_hypercall_table[eax].compat(ebx, ecx, edx, esi, edi,
                                                     ebp);
+        curr->hcall_compat = false;
 
 #ifndef NDEBUG
         if ( !curr->hcall_preempted )
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index 945afa0..c0718f8 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -234,7 +234,9 @@ void pv_hypercall(struct cpu_user_regs *regs)
             __trace_hypercall(TRC_PV_HYPERCALL_V2, eax, args);
         }
 
+        curr->hcall_compat = true;
         regs->_eax = pv_hypercall_table[eax].compat(ebx, ecx, edx, esi, edi, ebp);
+        curr->hcall_compat = false;
 
 #ifndef NDEBUG
         if ( !curr->hcall_preempted )
diff --git a/xen/include/asm-x86/hvm/vcpu.h b/xen/include/asm-x86/hvm/vcpu.h
index 6d5553d..6c54773 100644
--- a/xen/include/asm-x86/hvm/vcpu.h
+++ b/xen/include/asm-x86/hvm/vcpu.h
@@ -166,8 +166,6 @@ struct hvm_vcpu {
     bool                debug_state_latch;
     bool                single_step;
 
-    bool                hcall_64bit;
-
     struct hvm_vcpu_asid n1asid;
 
     u32                 msr_tsc_aux;
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 5b62238..738bb43 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -204,6 +204,10 @@ struct vcpu
     bool             affinity_broken;
     /* A hypercall has been preempted. */
     bool             hcall_preempted;
+#ifdef CONFIG_COMPAT
+    /* A hypercall is using the compat ABI? */
+    bool             hcall_compat;
+#endif
 
 
     /*
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] x86/hypercall: Split out PV hypercall infrastructure
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
                   ` (3 preceding siblings ...)
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_64bit boolean common Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 11:19   ` Jan Beulich
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Move hypercall continuation logic Andrew Cooper
  2017-02-15 19:41 ` [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM Andrew Cooper
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

Repurpose arch/x86/hypercall.c to be common x86 hypercall infrastructure, and
move the PV specific routines to arch/x86/pv/hypercall.c

This is purely code motion.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/Makefile             |   1 +
 xen/arch/x86/hypercall.c          | 226 +-------------------------------------
 xen/arch/x86/pv/Makefile          |   1 +
 xen/arch/x86/{ => pv}/hypercall.c |  59 +---------
 4 files changed, 8 insertions(+), 279 deletions(-)
 create mode 100644 xen/arch/x86/pv/Makefile
 copy xen/arch/x86/{ => pv}/hypercall.c (84%)

diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 007dced..10f519e 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -4,6 +4,7 @@ subdir-y += genapic
 subdir-y += hvm
 subdir-y += mm
 subdir-$(CONFIG_XENOPROF) += oprofile
+subdir-y += pv
 subdir-y += x86_64
 
 alternative-y := alternative.init.o
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index c0718f8..f807332 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -1,6 +1,8 @@
 /******************************************************************************
  * arch/x86/hypercall.c
  *
+ * Common x86 hypercall infrastructure.
+ *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; either version 2 of the License, or
@@ -17,9 +19,7 @@
  * Copyright (c) 2015,2016 Citrix Systems Ltd.
  */
 
-#include <xen/compiler.h>
 #include <xen/hypercall.h>
-#include <xen/trace.h>
 
 #define ARGS(x, n)                              \
     [ __HYPERVISOR_ ## x ] = { n, n }
@@ -74,228 +74,6 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
 #undef COMP
 #undef ARGS
 
-#define HYPERCALL(x)                                                \
-    [ __HYPERVISOR_ ## x ] = { (hypercall_fn_t *) do_ ## x,         \
-                               (hypercall_fn_t *) do_ ## x }
-#define COMPAT_CALL(x)                                              \
-    [ __HYPERVISOR_ ## x ] = { (hypercall_fn_t *) do_ ## x,         \
-                               (hypercall_fn_t *) compat_ ## x }
-
-#define do_arch_1             paging_domctl_continuation
-
-static const hypercall_table_t pv_hypercall_table[] = {
-    COMPAT_CALL(set_trap_table),
-    HYPERCALL(mmu_update),
-    COMPAT_CALL(set_gdt),
-    HYPERCALL(stack_switch),
-    COMPAT_CALL(set_callbacks),
-    HYPERCALL(fpu_taskswitch),
-    HYPERCALL(sched_op_compat),
-    COMPAT_CALL(platform_op),
-    HYPERCALL(set_debugreg),
-    HYPERCALL(get_debugreg),
-    COMPAT_CALL(update_descriptor),
-    COMPAT_CALL(memory_op),
-    COMPAT_CALL(multicall),
-    COMPAT_CALL(update_va_mapping),
-    COMPAT_CALL(set_timer_op),
-    HYPERCALL(event_channel_op_compat),
-    COMPAT_CALL(xen_version),
-    HYPERCALL(console_io),
-    COMPAT_CALL(physdev_op_compat),
-    COMPAT_CALL(grant_table_op),
-    COMPAT_CALL(vm_assist),
-    COMPAT_CALL(update_va_mapping_otherdomain),
-    COMPAT_CALL(iret),
-    COMPAT_CALL(vcpu_op),
-    HYPERCALL(set_segment_base),
-    COMPAT_CALL(mmuext_op),
-    COMPAT_CALL(xsm_op),
-    COMPAT_CALL(nmi_op),
-    COMPAT_CALL(sched_op),
-    COMPAT_CALL(callback_op),
-#ifdef CONFIG_XENOPROF
-    COMPAT_CALL(xenoprof_op),
-#endif
-    HYPERCALL(event_channel_op),
-    COMPAT_CALL(physdev_op),
-    HYPERCALL(hvm_op),
-    HYPERCALL(sysctl),
-    HYPERCALL(domctl),
-#ifdef CONFIG_KEXEC
-    COMPAT_CALL(kexec_op),
-#endif
-#ifdef CONFIG_TMEM
-    HYPERCALL(tmem_op),
-#endif
-    HYPERCALL(xenpmu_op),
-    COMPAT_CALL(dm_op),
-    HYPERCALL(mca),
-    HYPERCALL(arch_1),
-};
-
-#undef do_arch_1
-#undef COMPAT_CALL
-#undef HYPERCALL
-
-void pv_hypercall(struct cpu_user_regs *regs)
-{
-    struct vcpu *curr = current;
-    unsigned long eax;
-
-    ASSERT(guest_kernel_mode(curr, regs));
-
-    eax = is_pv_32bit_vcpu(curr) ? regs->_eax : regs->rax;
-
-    BUILD_BUG_ON(ARRAY_SIZE(pv_hypercall_table) >
-                 ARRAY_SIZE(hypercall_args_table));
-
-    if ( (eax >= ARRAY_SIZE(pv_hypercall_table)) ||
-         !pv_hypercall_table[eax].native )
-    {
-        regs->rax = -ENOSYS;
-        return;
-    }
-
-    curr->hcall_preempted = false;
-
-    if ( !is_pv_32bit_vcpu(curr) )
-    {
-        unsigned long rdi = regs->rdi;
-        unsigned long rsi = regs->rsi;
-        unsigned long rdx = regs->rdx;
-        unsigned long r10 = regs->r10;
-        unsigned long r8 = regs->r8;
-        unsigned long r9 = regs->r9;
-
-#ifndef NDEBUG
-        /* Deliberately corrupt parameter regs not used by this hypercall. */
-        switch ( hypercall_args_table[eax].native )
-        {
-        case 0: rdi = 0xdeadbeefdeadf00dUL;
-        case 1: rsi = 0xdeadbeefdeadf00dUL;
-        case 2: rdx = 0xdeadbeefdeadf00dUL;
-        case 3: r10 = 0xdeadbeefdeadf00dUL;
-        case 4: r8 = 0xdeadbeefdeadf00dUL;
-        case 5: r9 = 0xdeadbeefdeadf00dUL;
-        }
-#endif
-        if ( unlikely(tb_init_done) )
-        {
-            unsigned long args[6] = { rdi, rsi, rdx, r10, r8, r9 };
-
-            __trace_hypercall(TRC_PV_HYPERCALL_V2, eax, args);
-        }
-
-        regs->rax = pv_hypercall_table[eax].native(rdi, rsi, rdx, r10, r8, r9);
-
-#ifndef NDEBUG
-        if ( !curr->hcall_preempted )
-        {
-            /* Deliberately corrupt parameter regs used by this hypercall. */
-            switch ( hypercall_args_table[eax].native )
-            {
-            case 6: regs->r9  = 0xdeadbeefdeadf00dUL;
-            case 5: regs->r8  = 0xdeadbeefdeadf00dUL;
-            case 4: regs->r10 = 0xdeadbeefdeadf00dUL;
-            case 3: regs->rdx = 0xdeadbeefdeadf00dUL;
-            case 2: regs->rsi = 0xdeadbeefdeadf00dUL;
-            case 1: regs->rdi = 0xdeadbeefdeadf00dUL;
-            }
-        }
-#endif
-    }
-    else
-    {
-        unsigned int ebx = regs->_ebx;
-        unsigned int ecx = regs->_ecx;
-        unsigned int edx = regs->_edx;
-        unsigned int esi = regs->_esi;
-        unsigned int edi = regs->_edi;
-        unsigned int ebp = regs->_ebp;
-
-#ifndef NDEBUG
-        /* Deliberately corrupt parameter regs not used by this hypercall. */
-        switch ( hypercall_args_table[eax].compat )
-        {
-        case 0: ebx = 0xdeadf00d;
-        case 1: ecx = 0xdeadf00d;
-        case 2: edx = 0xdeadf00d;
-        case 3: esi = 0xdeadf00d;
-        case 4: edi = 0xdeadf00d;
-        case 5: ebp = 0xdeadf00d;
-        }
-#endif
-
-        if ( unlikely(tb_init_done) )
-        {
-            unsigned long args[6] = { ebx, ecx, edx, esi, edi, ebp };
-
-            __trace_hypercall(TRC_PV_HYPERCALL_V2, eax, args);
-        }
-
-        curr->hcall_compat = true;
-        regs->_eax = pv_hypercall_table[eax].compat(ebx, ecx, edx, esi, edi, ebp);
-        curr->hcall_compat = false;
-
-#ifndef NDEBUG
-        if ( !curr->hcall_preempted )
-        {
-            /* Deliberately corrupt parameter regs used by this hypercall. */
-            switch ( hypercall_args_table[eax].compat )
-            {
-            case 6: regs->_ebp = 0xdeadf00d;
-            case 5: regs->_edi = 0xdeadf00d;
-            case 4: regs->_esi = 0xdeadf00d;
-            case 3: regs->_edx = 0xdeadf00d;
-            case 2: regs->_ecx = 0xdeadf00d;
-            case 1: regs->_ebx = 0xdeadf00d;
-            }
-        }
-#endif
-    }
-
-    /*
-     * PV guests use SYSCALL or INT $0x82 to make a hypercall, both of which
-     * have trap semantics.  If the hypercall has been preempted, rewind the
-     * instruction pointer to reexecute the instruction.
-     */
-    if ( curr->hcall_preempted )
-        regs->rip -= 2;
-
-    perfc_incr(hypercalls);
-}
-
-void arch_do_multicall_call(struct mc_state *state)
-{
-    if ( !is_pv_32bit_vcpu(current) )
-    {
-        struct multicall_entry *call = &state->call;
-
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].native )
-            call->result = pv_hypercall_table[call->op].native(
-                call->args[0], call->args[1], call->args[2],
-                call->args[3], call->args[4], call->args[5]);
-        else
-            call->result = -ENOSYS;
-    }
-#ifdef CONFIG_COMPAT
-    else
-    {
-        struct compat_multicall_entry *call = &state->compat_call;
-
-        if ( (call->op < ARRAY_SIZE(pv_hypercall_table)) &&
-             pv_hypercall_table[call->op].compat )
-            call->result = pv_hypercall_table[call->op].compat(
-                call->args[0], call->args[1], call->args[2],
-                call->args[3], call->args[4], call->args[5]);
-        else
-            call->result = -ENOSYS;
-    }
-#endif
-}
-
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/x86/pv/Makefile b/xen/arch/x86/pv/Makefile
new file mode 100644
index 0000000..de21937
--- /dev/null
+++ b/xen/arch/x86/pv/Makefile
@@ -0,0 +1 @@
+obj-y += hypercall.o
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/pv/hypercall.c
similarity index 84%
copy from xen/arch/x86/hypercall.c
copy to xen/arch/x86/pv/hypercall.c
index c0718f8..5f5460a 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/pv/hypercall.c
@@ -1,5 +1,7 @@
 /******************************************************************************
- * arch/x86/hypercall.c
+ * arch/pv/hypercall.c
+ *
+ * PV hypercall dispatching routines
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -14,66 +16,13 @@
  * You should have received a copy of the GNU General Public License
  * along with this program; If not, see <http://www.gnu.org/licenses/>.
  *
- * Copyright (c) 2015,2016 Citrix Systems Ltd.
+ * Copyright (c) 2017 Citrix Systems Ltd.
  */
 
 #include <xen/compiler.h>
 #include <xen/hypercall.h>
 #include <xen/trace.h>
 
-#define ARGS(x, n)                              \
-    [ __HYPERVISOR_ ## x ] = { n, n }
-#define COMP(x, n, c)                           \
-    [ __HYPERVISOR_ ## x ] = { n, c }
-
-const hypercall_args_t hypercall_args_table[NR_hypercalls] =
-{
-    ARGS(set_trap_table, 1),
-    ARGS(mmu_update, 4),
-    ARGS(set_gdt, 2),
-    ARGS(stack_switch, 2),
-    COMP(set_callbacks, 3, 4),
-    ARGS(fpu_taskswitch, 1),
-    ARGS(sched_op_compat, 2),
-    ARGS(platform_op, 1),
-    ARGS(set_debugreg, 2),
-    ARGS(get_debugreg, 1),
-    COMP(update_descriptor, 2, 4),
-    ARGS(memory_op, 2),
-    ARGS(multicall, 2),
-    COMP(update_va_mapping, 3, 4),
-    COMP(set_timer_op, 1, 2),
-    ARGS(event_channel_op_compat, 1),
-    ARGS(xen_version, 2),
-    ARGS(console_io, 3),
-    ARGS(physdev_op_compat, 1),
-    ARGS(grant_table_op, 3),
-    ARGS(vm_assist, 2),
-    COMP(update_va_mapping_otherdomain, 4, 5),
-    ARGS(vcpu_op, 3),
-    COMP(set_segment_base, 2, 0),
-    ARGS(mmuext_op, 4),
-    ARGS(xsm_op, 1),
-    ARGS(nmi_op, 2),
-    ARGS(sched_op, 2),
-    ARGS(callback_op, 2),
-    ARGS(xenoprof_op, 2),
-    ARGS(event_channel_op, 2),
-    ARGS(physdev_op, 2),
-    ARGS(hvm_op, 2),
-    ARGS(sysctl, 1),
-    ARGS(domctl, 1),
-    ARGS(kexec_op, 2),
-    ARGS(tmem_op, 1),
-    ARGS(xenpmu_op, 2),
-    ARGS(dm_op, 3),
-    ARGS(mca, 1),
-    ARGS(arch_1, 1),
-};
-
-#undef COMP
-#undef ARGS
-
 #define HYPERCALL(x)                                                \
     [ __HYPERVISOR_ ## x ] = { (hypercall_fn_t *) do_ ## x,         \
                                (hypercall_fn_t *) do_ ## x }
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] x86/hypercall: Move hypercall continuation logic
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
                   ` (4 preceding siblings ...)
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Split out PV hypercall infrastructure Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 11:23   ` Jan Beulich
  2017-02-15 19:41 ` [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM Andrew Cooper
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

The newly-repurposed arch/x86/hypercall.c is a more appropriate place for the
hypercall continuation logic to live.

This is purely code motion.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/domain.c    | 177 -----------------------------------------------
 xen/arch/x86/hypercall.c | 177 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 177 insertions(+), 177 deletions(-)

diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
index 3209eb3..8d7cae3 100644
--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -2184,183 +2184,6 @@ void sync_vcpu_execstate(struct vcpu *v)
     flush_tlb_mask(v->vcpu_dirty_cpumask);
 }
 
-#define next_arg(fmt, args) ({                                              \
-    unsigned long __arg;                                                    \
-    switch ( *(fmt)++ )                                                     \
-    {                                                                       \
-    case 'i': __arg = (unsigned long)va_arg(args, unsigned int);  break;    \
-    case 'l': __arg = (unsigned long)va_arg(args, unsigned long); break;    \
-    case 'h': __arg = (unsigned long)va_arg(args, void *);        break;    \
-    default:  __arg = 0; BUG();                                             \
-    }                                                                       \
-    __arg;                                                                  \
-})
-
-void hypercall_cancel_continuation(void)
-{
-    current->hcall_preempted = false;
-}
-
-unsigned long hypercall_create_continuation(
-    unsigned int op, const char *format, ...)
-{
-    struct vcpu *curr = current;
-    struct mc_state *mcs = &curr->mc_state;
-    const char *p = format;
-    unsigned long arg;
-    unsigned int i;
-    va_list args;
-
-    curr->hcall_preempted = true;
-
-    va_start(args, format);
-
-    if ( mcs->flags & MCSF_in_multicall )
-    {
-        for ( i = 0; *p != '\0'; i++ )
-            mcs->call.args[i] = next_arg(p, args);
-    }
-    else
-    {
-        struct cpu_user_regs *regs = guest_cpu_user_regs();
-
-        regs->rax = op;
-
-        if ( !curr->hcall_compat )
-        {
-            for ( i = 0; *p != '\0'; i++ )
-            {
-                arg = next_arg(p, args);
-                switch ( i )
-                {
-                case 0: regs->rdi = arg; break;
-                case 1: regs->rsi = arg; break;
-                case 2: regs->rdx = arg; break;
-                case 3: regs->r10 = arg; break;
-                case 4: regs->r8  = arg; break;
-                case 5: regs->r9  = arg; break;
-                }
-            }
-        }
-        else
-        {
-            for ( i = 0; *p != '\0'; i++ )
-            {
-                arg = next_arg(p, args);
-                switch ( i )
-                {
-                case 0: regs->rbx = arg; break;
-                case 1: regs->rcx = arg; break;
-                case 2: regs->rdx = arg; break;
-                case 3: regs->rsi = arg; break;
-                case 4: regs->rdi = arg; break;
-                case 5: regs->rbp = arg; break;
-                }
-            }
-        }
-    }
-
-    va_end(args);
-
-    return op;
-}
-
-int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
-                                unsigned int mask, ...)
-{
-    int rc = 0;
-    struct mc_state *mcs = &current->mc_state;
-    struct cpu_user_regs *regs;
-    unsigned int i, cval = 0;
-    unsigned long nval = 0;
-    va_list args;
-
-    ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
-    ASSERT(!(mask >> nr));
-    ASSERT(!id || *id < nr);
-    ASSERT(!id || !(mask & (1U << *id)));
-
-    va_start(args, mask);
-
-    if ( mcs->flags & MCSF_in_multicall )
-    {
-        if ( !current->hcall_preempted )
-        {
-            va_end(args);
-            return 0;
-        }
-
-        for ( i = 0; i < nr; ++i, mask >>= 1 )
-        {
-            if ( mask & 1 )
-            {
-                nval = va_arg(args, unsigned long);
-                cval = va_arg(args, unsigned int);
-                if ( cval == nval )
-                    mask &= ~1U;
-                else
-                    BUG_ON(nval == (unsigned int)nval);
-            }
-            else if ( id && *id == i )
-            {
-                *id = mcs->call.args[i];
-                id = NULL;
-            }
-            if ( (mask & 1) && mcs->call.args[i] == nval )
-            {
-                mcs->call.args[i] = cval;
-                ++rc;
-            }
-            else
-                BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
-        }
-    }
-    else
-    {
-        regs = guest_cpu_user_regs();
-        for ( i = 0; i < nr; ++i, mask >>= 1 )
-        {
-            unsigned long *reg;
-
-            switch ( i )
-            {
-            case 0: reg = &regs->rbx; break;
-            case 1: reg = &regs->rcx; break;
-            case 2: reg = &regs->rdx; break;
-            case 3: reg = &regs->rsi; break;
-            case 4: reg = &regs->rdi; break;
-            case 5: reg = &regs->rbp; break;
-            default: BUG(); reg = NULL; break;
-            }
-            if ( (mask & 1) )
-            {
-                nval = va_arg(args, unsigned long);
-                cval = va_arg(args, unsigned int);
-                if ( cval == nval )
-                    mask &= ~1U;
-                else
-                    BUG_ON(nval == (unsigned int)nval);
-            }
-            else if ( id && *id == i )
-            {
-                *id = *reg;
-                id = NULL;
-            }
-            if ( (mask & 1) && *reg == nval )
-            {
-                *reg = cval;
-                ++rc;
-            }
-            else
-                BUG_ON(*reg != (unsigned int)*reg);
-        }
-    }
-
-    va_end(args);
-
-    return rc;
-}
-
 static int relinquish_memory(
     struct domain *d, struct page_list_head *list, unsigned long type)
 {
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index f807332..e301818 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -74,6 +74,183 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
 #undef COMP
 #undef ARGS
 
+#define next_arg(fmt, args) ({                                              \
+    unsigned long __arg;                                                    \
+    switch ( *(fmt)++ )                                                     \
+    {                                                                       \
+    case 'i': __arg = (unsigned long)va_arg(args, unsigned int);  break;    \
+    case 'l': __arg = (unsigned long)va_arg(args, unsigned long); break;    \
+    case 'h': __arg = (unsigned long)va_arg(args, void *);        break;    \
+    default:  __arg = 0; BUG();                                             \
+    }                                                                       \
+    __arg;                                                                  \
+})
+
+void hypercall_cancel_continuation(void)
+{
+    current->hcall_preempted = false;
+}
+
+unsigned long hypercall_create_continuation(
+    unsigned int op, const char *format, ...)
+{
+    struct vcpu *curr = current;
+    struct mc_state *mcs = &curr->mc_state;
+    const char *p = format;
+    unsigned long arg;
+    unsigned int i;
+    va_list args;
+
+    curr->hcall_preempted = true;
+
+    va_start(args, format);
+
+    if ( mcs->flags & MCSF_in_multicall )
+    {
+        for ( i = 0; *p != '\0'; i++ )
+            mcs->call.args[i] = next_arg(p, args);
+    }
+    else
+    {
+        struct cpu_user_regs *regs = guest_cpu_user_regs();
+
+        regs->rax = op;
+
+        if ( !curr->hcall_compat )
+        {
+            for ( i = 0; *p != '\0'; i++ )
+            {
+                arg = next_arg(p, args);
+                switch ( i )
+                {
+                case 0: regs->rdi = arg; break;
+                case 1: regs->rsi = arg; break;
+                case 2: regs->rdx = arg; break;
+                case 3: regs->r10 = arg; break;
+                case 4: regs->r8  = arg; break;
+                case 5: regs->r9  = arg; break;
+                }
+            }
+        }
+        else
+        {
+            for ( i = 0; *p != '\0'; i++ )
+            {
+                arg = next_arg(p, args);
+                switch ( i )
+                {
+                case 0: regs->rbx = arg; break;
+                case 1: regs->rcx = arg; break;
+                case 2: regs->rdx = arg; break;
+                case 3: regs->rsi = arg; break;
+                case 4: regs->rdi = arg; break;
+                case 5: regs->rbp = arg; break;
+                }
+            }
+        }
+    }
+
+    va_end(args);
+
+    return op;
+}
+
+int hypercall_xlat_continuation(unsigned int *id, unsigned int nr,
+                                unsigned int mask, ...)
+{
+    int rc = 0;
+    struct mc_state *mcs = &current->mc_state;
+    struct cpu_user_regs *regs;
+    unsigned int i, cval = 0;
+    unsigned long nval = 0;
+    va_list args;
+
+    ASSERT(nr <= ARRAY_SIZE(mcs->call.args));
+    ASSERT(!(mask >> nr));
+    ASSERT(!id || *id < nr);
+    ASSERT(!id || !(mask & (1U << *id)));
+
+    va_start(args, mask);
+
+    if ( mcs->flags & MCSF_in_multicall )
+    {
+        if ( !current->hcall_preempted )
+        {
+            va_end(args);
+            return 0;
+        }
+
+        for ( i = 0; i < nr; ++i, mask >>= 1 )
+        {
+            if ( mask & 1 )
+            {
+                nval = va_arg(args, unsigned long);
+                cval = va_arg(args, unsigned int);
+                if ( cval == nval )
+                    mask &= ~1U;
+                else
+                    BUG_ON(nval == (unsigned int)nval);
+            }
+            else if ( id && *id == i )
+            {
+                *id = mcs->call.args[i];
+                id = NULL;
+            }
+            if ( (mask & 1) && mcs->call.args[i] == nval )
+            {
+                mcs->call.args[i] = cval;
+                ++rc;
+            }
+            else
+                BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
+        }
+    }
+    else
+    {
+        regs = guest_cpu_user_regs();
+        for ( i = 0; i < nr; ++i, mask >>= 1 )
+        {
+            unsigned long *reg;
+
+            switch ( i )
+            {
+            case 0: reg = &regs->rbx; break;
+            case 1: reg = &regs->rcx; break;
+            case 2: reg = &regs->rdx; break;
+            case 3: reg = &regs->rsi; break;
+            case 4: reg = &regs->rdi; break;
+            case 5: reg = &regs->rbp; break;
+            default: BUG(); reg = NULL; break;
+            }
+            if ( (mask & 1) )
+            {
+                nval = va_arg(args, unsigned long);
+                cval = va_arg(args, unsigned int);
+                if ( cval == nval )
+                    mask &= ~1U;
+                else
+                    BUG_ON(nval == (unsigned int)nval);
+            }
+            else if ( id && *id == i )
+            {
+                *id = *reg;
+                id = NULL;
+            }
+            if ( (mask & 1) && *reg == nval )
+            {
+                *reg = cval;
+                ++rc;
+            }
+            else
+                BUG_ON(*reg != (unsigned int)*reg);
+        }
+    }
+
+    va_end(args);
+
+    return rc;
+}
+
 /*
  * Local variables:
  * mode: C
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM
  2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
                   ` (5 preceding siblings ...)
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Move hypercall continuation logic Andrew Cooper
@ 2017-02-15 19:41 ` Andrew Cooper
  2017-02-16 14:39   ` Jan Beulich
  6 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-15 19:41 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Jan Beulich

Making PV and HVM guests individually compilable is useful as a reduction in
hypervisor size, and as an aid to enforcing clean API boundaries.

Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
until either can actually be disabled.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
---
 xen/arch/x86/Kconfig               | 68 ++++++++++++++++++++++++++++----------
 xen/arch/x86/Makefile              |  4 +--
 xen/tools/kconfig/allrandom.config |  4 +++
 3 files changed, 56 insertions(+), 20 deletions(-)

diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 96ca2bf..87b03f3 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -32,6 +32,56 @@ menu "Architecture Features"
 
 source "arch/Kconfig"
 
+config PV
+	def_bool y
+	prompt "PV guest support" if EXPERT = "y"
+	depends on X86
+	---help---
+	  Support for Xen Paravirtual guests.
+
+	  Xen PV guests use Ring Deprivileging as a method of virtualisation.
+	  This requires no specific hardware support, but does require an OS
+	  capable of running PV under Xen.
+
+	  If unsure, say Y.
+
+	  TODO: This is a work in process, and Xen doesn't currently compile
+	  if this option is disabled.
+
+config HVM
+	def_bool y
+	prompt "HVM guest support" if EXPERT = "y"
+	depends on X86
+	---help---
+	  Support for Xen Hardware Virtual Machine guests.
+
+	  Xen HVM guests use hardware virtualisation extentions (Intel VT-x,
+	  AMD SVM) to run OSes.	 This requires a CPU with appropriate
+	  extentions, but can in principle run any OS, even if unmodified.
+
+	  If unsure, say Y.
+
+	  TODO: This is a work in process, and Xen doesn't currently compile
+	  if this option is disabled.
+
+config HVM_FEP
+	bool "HVM Forced Emulation Prefix support" if EXPERT = "y"
+	default DEBUG
+	depends on HVM
+	---help---
+	  Compiles in a feature that allows HVM guest to arbitrarily
+	  exercise the instruction emulator.
+
+	  This feature can only be enabled during boot time with
+	  appropriate hypervisor command line option. Please read
+	  hypervisor command line documentation before trying to use
+	  this feature.
+
+	  This is strictly for testing purposes, and not appropriate
+	  for use in production.
+
+	  If unsure, say N.
+
 config SHADOW_PAGING
         bool "Shadow Paging"
         default y
@@ -61,24 +111,6 @@ config BIGMEM
 
 	  If unsure, say N.
 
-config HVM_FEP
-	bool "HVM Forced Emulation Prefix support" if EXPERT = "y"
-	default DEBUG
-	---help---
-
-	  Compiles in a feature that allows HVM guest to arbitrarily
-	  exercise the instruction emulator.
-
-	  This feature can only be enabled during boot time with
-	  appropriate hypervisor command line option. Please read
-	  hypervisor command line documentation before trying to use
-	  this feature.
-
-	  This is strictly for testing purposes, and not appropriate
-	  for use in production.
-
-	  If unsure, say N.
-
 config TBOOT
 	def_bool y
 	prompt "Xen tboot support" if EXPERT = "y"
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index 10f519e..d7c8970 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -1,10 +1,10 @@
 subdir-y += acpi
 subdir-y += cpu
 subdir-y += genapic
-subdir-y += hvm
+subdir-$(CONFIG_HVM) += hvm
 subdir-y += mm
 subdir-$(CONFIG_XENOPROF) += oprofile
-subdir-y += pv
+subdir-$(CONFIG_PV) += pv
 subdir-y += x86_64
 
 alternative-y := alternative.init.o
diff --git a/xen/tools/kconfig/allrandom.config b/xen/tools/kconfig/allrandom.config
index c7753ac..329ef59 100644
--- a/xen/tools/kconfig/allrandom.config
+++ b/xen/tools/kconfig/allrandom.config
@@ -1,3 +1,7 @@
 # Explicit option choices not subject to regular RANDCONFIG
 
 CONFIG_GCOV_FORMAT_AUTODETECT=y
+
+# Being able to compile-out guest subsystems is a work-in-progress.
+CONFIG_PV=y
+CONFIG_HVM=y
-- 
2.1.4


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

^ permalink raw reply related	[flat|nested] 20+ messages in thread
* Re: [PATCH] xen/multicall: Use the common hcall_preempted boolean
  2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
@ 2017-02-16 10:37   ` Jan Beulich
  2017-02-16 10:42     ` Andrew Cooper
  2017-02-16 11:02   ` [PATCH 2/7] " Jan Beulich
  2017-02-16 12:10   ` [PATCH] " Julien Grall
  2 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 10:37 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: JulienGrall, Stefano Stabellini, Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> The now-common hcall_preempted boolean is perfectly usable for multicalls.

Something must have gone in the wrong order here: This is not part
of a series, and I can't see hcall_preempted being common in
staging.

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] xen/multicall: Use the common hcall_preempted boolean
  2017-02-16 10:37   ` Jan Beulich
@ 2017-02-16 10:42     ` Andrew Cooper
  0 siblings, 0 replies; 20+ messages in thread
From: Andrew Cooper @ 2017-02-16 10:42 UTC (permalink / raw)
  To: Jan Beulich; +Cc: JulienGrall, Stefano Stabellini, Xen-devel

On 16/02/17 10:37, Jan Beulich wrote:
>>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
>> The now-common hcall_preempted boolean is perfectly usable for multicalls.
> Something must have gone in the wrong order here: This is not part
> of a series, and I can't see hcall_preempted being common in
> staging.

Sorry.  I fumbled `git format-patch` and omitted the patch numbers.

The correct order of the series is:

Andrew Cooper (7):
  x86/hypercall: Make the HVM hcall_preempted boolean common
  arm/hypercall: Use the common hcall_preempted boolean
  xen/multicall: Use the common hcall_preempted boolean
  x86/hypercall: Make the HVM hcall_64bit boolean common
  x86/hypercall: Split out PV hypercall infrastructure
  x86/hypercall: Move hypercall continuation logic
  [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM

~Andrew

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 1/7] x86/hypercall: Make the HVM hcall_preempted boolean common
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common Andrew Cooper
@ 2017-02-16 10:44   ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 10:44 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/include/xen/sched.h
> +++ b/xen/include/xen/sched.h
> @@ -202,6 +202,8 @@ struct vcpu
>      bool             paused_for_shutdown;
>      /* VCPU need affinity restored */
>      bool             affinity_broken;
> +    /* A hypercall has been preempted. */
> +    bool             hcall_preempted;
>  
>  
>      /*

Would you mind moving this addition between the two existing blank
lines, so that together with the later movement here of hcall_64bit
the two appear as a proper pair? With that

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 2/7] xen/multicall: Use the common hcall_preempted boolean
  2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
  2017-02-16 10:37   ` Jan Beulich
@ 2017-02-16 11:02   ` Jan Beulich
  2017-02-16 12:10   ` [PATCH] " Julien Grall
  2 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 11:02 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: JulienGrall, Stefano Stabellini, Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> The now-common hcall_preempted boolean is perfectly usable for multicalls.
> Remove the multicall-specific preemption mechanism.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH 4/7] x86/hypercall: Make the HVM hcall_64bit boolean common
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_64bit boolean common Andrew Cooper
@ 2017-02-16 11:07   ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 11:07 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> HVM guests currently make use of arch.hvm_vcpu.hcall_64bit to track the ABI of
> the hypercall in use.
> 
> The rest of Xen deals in terms of the comat ABI or not, so rename the boolean
> and make it common, guared by CONFIG_COMPAT to avoid bloat if a compat ABI is
> not wanted/needed.
> 
> Set hcall_compat uniformly for PV guests as well as HVM guests.  This removes
> the remaining piece of guest-type-specific knowledge from
> hypercall_create_continuation(), allowing it to operate only in terms of the
> hypercall ABI in use.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>



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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] x86/hypercall: Split out PV hypercall infrastructure
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Split out PV hypercall infrastructure Andrew Cooper
@ 2017-02-16 11:19   ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 11:19 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/hypercall.c
> +++ b/xen/arch/x86/pv/hypercall.c
> @@ -1,5 +1,7 @@
> /******************************************************************************
> - * arch/x86/hypercall.c
> + * arch/pv/hypercall.c

arch/x86/pv/hypercall.c (if we really need files to name themselves)

Acked-by: Jan Beulich <jbeulich@suse.com>



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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] x86/hypercall: Move hypercall continuation logic
  2017-02-15 19:41 ` [PATCH] x86/hypercall: Move hypercall continuation logic Andrew Cooper
@ 2017-02-16 11:23   ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 11:23 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> The newly-repurposed arch/x86/hypercall.c is a more appropriate place for the
> hypercall continuation logic to live.
> 
> This is purely code motion.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Acked-by: Jan Beulich <jbeulich@suse.com>



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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] arm/hypercall: Use the common hcall_preempted boolean
  2017-02-15 19:41 ` [PATCH] arm/hypercall: Use the common hcall_preempted boolean Andrew Cooper
@ 2017-02-16 12:04   ` Julien Grall
  0 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2017-02-16 12:04 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: nd, Stefano Stabellini

Hi Andrew,

On 15/02/17 19:41, Andrew Cooper wrote:
> With hcall_preempted having just been made common, ARM can use use it to
> simplify its hypercall handling.
>
> This simplifies the continuation logic and removes the risk of accidentally
> skipping multiple instructions.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] xen/multicall: Use the common hcall_preempted boolean
  2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
  2017-02-16 10:37   ` Jan Beulich
  2017-02-16 11:02   ` [PATCH 2/7] " Jan Beulich
@ 2017-02-16 12:10   ` Julien Grall
  2 siblings, 0 replies; 20+ messages in thread
From: Julien Grall @ 2017-02-16 12:10 UTC (permalink / raw)
  To: Andrew Cooper, Xen-devel; +Cc: nd, Stefano Stabellini, Jan Beulich

Hi Andrew,

On 15/02/17 19:41, Andrew Cooper wrote:
> The now-common hcall_preempted boolean is perfectly usable for multicalls.
> Remove the multicall-specific preemption mechanism.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

For the ARM bits:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM
  2017-02-15 19:41 ` [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM Andrew Cooper
@ 2017-02-16 14:39   ` Jan Beulich
  2017-02-16 14:58     ` Andrew Cooper
  0 siblings, 1 reply; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 14:39 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
> Making PV and HVM guests individually compilable is useful as a reduction in
> hypervisor size, and as an aid to enforcing clean API boundaries.
> 
> Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
> until either can actually be disabled.

While this is a nice goal, is it really useful to add these config options
now, rather than when that lot of work is at least near completion?
Or otherwise, how about making them prompt-less for now even for
EXPERT?

> --- a/xen/arch/x86/Kconfig
> +++ b/xen/arch/x86/Kconfig
> @@ -32,6 +32,56 @@ menu "Architecture Features"
>  
>  source "arch/Kconfig"
>  
> +config PV
> +	def_bool y
> +	prompt "PV guest support" if EXPERT = "y"
> +	depends on X86
> +	---help---
> +	  Support for Xen Paravirtual guests.
> +
> +	  Xen PV guests use Ring Deprivileging as a method of virtualisation.
> +	  This requires no specific hardware support, but does require an OS
> +	  capable of running PV under Xen.
> +
> +	  If unsure, say Y.
> +
> +	  TODO: This is a work in process, and Xen doesn't currently compile
> +	  if this option is disabled.
> +
> +config HVM
> +	def_bool y
> +	prompt "HVM guest support" if EXPERT = "y"
> +	depends on X86

Both of them getting selected to "no" makes extremely little sense,
I think, so I would wish to have a guard against that. Maybe the
user visible part wants to be a choice (both, PV only, HVM only),
selecting prompt-less PV and HVM as necessary?

Furthermore, considering what file they're in, I don't think the
"depends on X86" are necessary here (oddly enough TBOOT has
this too, but SHADOW_PAGING and BIGMEM don't).

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM
  2017-02-16 14:39   ` Jan Beulich
@ 2017-02-16 14:58     ` Andrew Cooper
  2017-02-16 15:49       ` Jan Beulich
  0 siblings, 1 reply; 20+ messages in thread
From: Andrew Cooper @ 2017-02-16 14:58 UTC (permalink / raw)
  To: Jan Beulich; +Cc: Xen-devel

On 16/02/17 14:39, Jan Beulich wrote:
>>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
>> Making PV and HVM guests individually compilable is useful as a reduction in
>> hypervisor size, and as an aid to enforcing clean API boundaries.
>>
>> Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
>> until either can actually be disabled.
> While this is a nice goal, is it really useful to add these config options
> now, rather than when that lot of work is at least near completion?

Hence the RFC on this patch.

> Or otherwise, how about making them prompt-less for now even for
> EXPERT?

Doesn't that prohibit them from being altered, even for development
purposes?

>
>> --- a/xen/arch/x86/Kconfig
>> +++ b/xen/arch/x86/Kconfig
>> @@ -32,6 +32,56 @@ menu "Architecture Features"
>>  
>>  source "arch/Kconfig"
>>  
>> +config PV
>> +	def_bool y
>> +	prompt "PV guest support" if EXPERT = "y"
>> +	depends on X86
>> +	---help---
>> +	  Support for Xen Paravirtual guests.
>> +
>> +	  Xen PV guests use Ring Deprivileging as a method of virtualisation.
>> +	  This requires no specific hardware support, but does require an OS
>> +	  capable of running PV under Xen.
>> +
>> +	  If unsure, say Y.
>> +
>> +	  TODO: This is a work in process, and Xen doesn't currently compile
>> +	  if this option is disabled.
>> +
>> +config HVM
>> +	def_bool y
>> +	prompt "HVM guest support" if EXPERT = "y"
>> +	depends on X86
> Both of them getting selected to "no" makes extremely little sense,
> I think, so I would wish to have a guard against that. Maybe the
> user visible part wants to be a choice (both, PV only, HVM only),
> selecting prompt-less PV and HVM as necessary?

There are a few edgecases whether neither would be useful, e.g. seeing
if we have subsystems in the resulting compile which should be covered
under PV or HVM.  I also had occason to need no guests whatsoever when
doing the HPET work (and I still have the debugging patch to allow Xen
to run without a dom0).

As this is already under EXPERT (and I expect that not to change), I
don't it is too much of a problem allowing both to be disabled.

> Furthermore, considering what file they're in, I don't think the
> "depends on X86" are necessary here (oddly enough TBOOT has
> this too, but SHADOW_PAGING and BIGMEM don't).

I did find this weird.

~Andrew

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

^ permalink raw reply	[flat|nested] 20+ messages in thread
* Re: [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM
  2017-02-16 14:58     ` Andrew Cooper
@ 2017-02-16 15:49       ` Jan Beulich
  0 siblings, 0 replies; 20+ messages in thread
From: Jan Beulich @ 2017-02-16 15:49 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel

>>> On 16.02.17 at 15:58, <andrew.cooper3@citrix.com> wrote:
> On 16/02/17 14:39, Jan Beulich wrote:
>>>>> On 15.02.17 at 20:41, <andrew.cooper3@citrix.com> wrote:
>>> Making PV and HVM guests individually compilable is useful as a reduction in
>>> hypervisor size, and as an aid to enforcing clean API boundaries.
>>>
>>> Introduce CONFIG_PV and CONFIG_HVM, although there is a lot of work to do
>>> until either can actually be disabled.
>> While this is a nice goal, is it really useful to add these config options
>> now, rather than when that lot of work is at least near completion?
> 
> Hence the RFC on this patch.
> 
>> Or otherwise, how about making them prompt-less for now even for
>> EXPERT?
> 
> Doesn't that prohibit them from being altered, even for development
> purposes?

Well, one would have to add a prompt line for the value to be
changeable. But that would be a 1-line change, much smaller
compared to the patch here, and hence acceptable imo.

>>> --- a/xen/arch/x86/Kconfig
>>> +++ b/xen/arch/x86/Kconfig
>>> @@ -32,6 +32,56 @@ menu "Architecture Features"
>>>  
>>>  source "arch/Kconfig"
>>>  
>>> +config PV
>>> +	def_bool y
>>> +	prompt "PV guest support" if EXPERT = "y"
>>> +	depends on X86
>>> +	---help---
>>> +	  Support for Xen Paravirtual guests.
>>> +
>>> +	  Xen PV guests use Ring Deprivileging as a method of virtualisation.
>>> +	  This requires no specific hardware support, but does require an OS
>>> +	  capable of running PV under Xen.
>>> +
>>> +	  If unsure, say Y.
>>> +
>>> +	  TODO: This is a work in process, and Xen doesn't currently compile
>>> +	  if this option is disabled.
>>> +
>>> +config HVM
>>> +	def_bool y
>>> +	prompt "HVM guest support" if EXPERT = "y"
>>> +	depends on X86
>> Both of them getting selected to "no" makes extremely little sense,
>> I think, so I would wish to have a guard against that. Maybe the
>> user visible part wants to be a choice (both, PV only, HVM only),
>> selecting prompt-less PV and HVM as necessary?
> 
> There are a few edgecases whether neither would be useful, e.g. seeing
> if we have subsystems in the resulting compile which should be covered
> under PV or HVM.  I also had occason to need no guests whatsoever when
> doing the HPET work (and I still have the debugging patch to allow Xen
> to run without a dom0).

Oh, okay.

>> Furthermore, considering what file they're in, I don't think the
>> "depends on X86" are necessary here (oddly enough TBOOT has
>> this too, but SHADOW_PAGING and BIGMEM don't).
> 
> I did find this weird.

Which part - the pointlessly present one, or the absent ones?

Jan


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

^ permalink raw reply	[flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-02-16 15:49 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15 19:41 [PATCH] Common hypercall handing improvements Andrew Cooper
2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_preempted boolean common Andrew Cooper
2017-02-16 10:44   ` [PATCH 1/7] " Jan Beulich
2017-02-15 19:41 ` [PATCH] arm/hypercall: Use the common hcall_preempted boolean Andrew Cooper
2017-02-16 12:04   ` Julien Grall
2017-02-15 19:41 ` [PATCH] xen/multicall: " Andrew Cooper
2017-02-16 10:37   ` Jan Beulich
2017-02-16 10:42     ` Andrew Cooper
2017-02-16 11:02   ` [PATCH 2/7] " Jan Beulich
2017-02-16 12:10   ` [PATCH] " Julien Grall
2017-02-15 19:41 ` [PATCH] x86/hypercall: Make the HVM hcall_64bit boolean common Andrew Cooper
2017-02-16 11:07   ` [PATCH 4/7] " Jan Beulich
2017-02-15 19:41 ` [PATCH] x86/hypercall: Split out PV hypercall infrastructure Andrew Cooper
2017-02-16 11:19   ` Jan Beulich
2017-02-15 19:41 ` [PATCH] x86/hypercall: Move hypercall continuation logic Andrew Cooper
2017-02-16 11:23   ` Jan Beulich
2017-02-15 19:41 ` [PATCH] [RFC] x86/kconfig: Introduce CONFIG_PV and CONFIG_HVM Andrew Cooper
2017-02-16 14:39   ` Jan Beulich
2017-02-16 14:58     ` Andrew Cooper
2017-02-16 15:49       ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.