Re: [Xen-devel] [PATCH] xen: lift hypercall_cancel_continuation to sched.h

[Jan Beulich]

>>> On 14.08.17 at 17:46,  wrote:
> --- a/xen/arch/x86/x86_64/compat/mm.c
> +++ b/xen/arch/x86/x86_64/compat/mm.c
> @@ -53,6 +53,7 @@ int compat_arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  compat_pfn_t mfn;
>  unsigned int i;
>  int rc = 0;
> +struct vcpu *curr = current;
>  
>  switch ( cmd )
>  {
> @@ -124,7 +125,7 @@ int compat_arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  if ( __copy_to_guest(arg, , 1) )
>  {
>  if ( rc == __HYPERVISOR_memory_op )
> -hypercall_cancel_continuation();
> +hypercall_cancel_continuation(curr);
>  rc = -EFAULT;
>  }
>  
> @@ -133,7 +134,7 @@ int compat_arch_memory_op(unsigned long cmd, 
> XEN_GUEST_HANDLE_PARAM(void) arg)
>  
>  case XENMEM_machphys_mapping:
>  {
> -struct domain *d = current->domain;
> +struct domain *d = curr->domain;
>  struct compat_machphys_mapping mapping = {
>  .v_start = MACH2PHYS_COMPAT_VIRT_START(d),
>  .v_end   = MACH2PHYS_COMPAT_VIRT_END,

I don't think the local variable is really useful here - it's two
entirely distinct and uncommon paths which need it, yet
you calculate unconditionally now. With the variable
dropped
Reviewed-by: Jan Beulich 

Jan


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

Re: [Xen-devel] [PATCH] xen: lift hypercall_cancel_continuation to sched.h

[Julien Grall]

Hi Wei,

On 14/08/17 16:46, Wei Liu wrote:

The function is the same on both x86 and arm. Lift it to sched.h to
save a function call, make it take a pointer to vcpu to avoid
resolving current every time it gets called.

Take the chance to change its callers to only use one current in code.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Stefano Stabellini 
Cc: Julien Grall 


For ARM bits:

Acked-by: Julien Grall 

Cheers,

--
Julien Grall

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

[Xen-devel] [PATCH] xen: lift hypercall_cancel_continuation to sched.h

[Wei Liu]

The function is the same on both x86 and arm. Lift it to sched.h to
save a function call, make it take a pointer to vcpu to avoid
resolving current every time it gets called.

Take the chance to change its callers to only use one current in code.

Signed-off-by: Wei Liu 
---
Cc: Jan Beulich 
Cc: Andrew Cooper 
Cc: Stefano Stabellini 
Cc: Julien Grall 
---
 xen/arch/arm/domain.c   | 5 -
 xen/arch/x86/hypercall.c| 5 -
 xen/arch/x86/x86_64/compat/mm.c | 5 +++--
 xen/common/multicall.c  | 7 ---
 xen/include/xen/sched.h | 6 +-
 5 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 2dc8b0ab5a..eeebbdb39a 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -365,11 +365,6 @@ void sync_vcpu_execstate(struct vcpu *v)
 __arg;  \
 })
 
-void hypercall_cancel_continuation(void)
-{
-current->hcall_preempted = false;
-}
-
 unsigned long hypercall_create_continuation(
 unsigned int op, const char *format, ...)
 {
diff --git a/xen/arch/x86/hypercall.c b/xen/arch/x86/hypercall.c
index e30181817a..90e88c1d2c 100644
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -86,11 +86,6 @@ const hypercall_args_t hypercall_args_table[NR_hypercalls] =
 __arg;  \
 })
 
-void hypercall_cancel_continuation(void)
-{
-current->hcall_preempted = false;
-}
-
 unsigned long hypercall_create_continuation(
 unsigned int op, const char *format, ...)
 {
diff --git a/xen/arch/x86/x86_64/compat/mm.c b/xen/arch/x86/x86_64/compat/mm.c
index b737af1888..f3fc07c3a9 100644
--- a/xen/arch/x86/x86_64/compat/mm.c
+++ b/xen/arch/x86/x86_64/compat/mm.c
@@ -53,6 +53,7 @@ int compat_arch_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 compat_pfn_t mfn;
 unsigned int i;
 int rc = 0;
+struct vcpu *curr = current;
 
 switch ( cmd )
 {
@@ -124,7 +125,7 @@ int compat_arch_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 if ( __copy_to_guest(arg, , 1) )
 {
 if ( rc == __HYPERVISOR_memory_op )
-hypercall_cancel_continuation();
+hypercall_cancel_continuation(curr);
 rc = -EFAULT;
 }
 
@@ -133,7 +134,7 @@ int compat_arch_memory_op(unsigned long cmd, 
XEN_GUEST_HANDLE_PARAM(void) arg)
 
 case XENMEM_machphys_mapping:
 {
-struct domain *d = current->domain;
+struct domain *d = curr->domain;
 struct compat_machphys_mapping mapping = {
 .v_start = MACH2PHYS_COMPAT_VIRT_START(d),
 .v_end   = MACH2PHYS_COMPAT_VIRT_END,
diff --git a/xen/common/multicall.c b/xen/common/multicall.c
index 7cbf857048..c7af4e0156 100644
--- a/xen/common/multicall.c
+++ b/xen/common/multicall.c
@@ -36,7 +36,8 @@ ret_t
 do_multicall(
 XEN_GUEST_HANDLE_PARAM(multicall_entry_t) call_list, uint32_t nr_calls)
 {
-struct mc_state *mcs = >mc_state;
+struct vcpu *curr = current;
+struct mc_state *mcs = >mc_state;
 uint32_t i;
 int  rc = 0;
 enum mc_disposition disp = mc_continue;
@@ -86,7 +87,7 @@ do_multicall(
 else if ( unlikely(__copy_field_to_guest(call_list, >call,
  result)) )
 rc = -EFAULT;
-else if ( current->hcall_preempted )
+else if ( curr->hcall_preempted )
 {
 /* Translate sub-call continuation to guest layout */
 xlat_multicall_entry(mcs);
@@ -95,7 +96,7 @@ do_multicall(
 if ( likely(!__copy_to_guest(call_list, >call, 1)) )
 goto preempted;
 else
-hypercall_cancel_continuation();
+hypercall_cancel_continuation(curr);
 rc = -EFAULT;
 }
 else
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 6673b27d88..4b239452c8 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -705,7 +705,11 @@ extern void (*dead_idle) (void);
  */
 unsigned long hypercall_create_continuation(
 unsigned int op, const char *format, ...);
-void hypercall_cancel_continuation(void);
+
+static inline void hypercall_cancel_continuation(struct vcpu *v)
+{
+v->hcall_preempted = false;
+}
 
 /*
  * For long-running operations that must be in hypercall context, check
-- 
2.11.0


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