Re: [Xen-devel] [PATCH v4 06/31] x86: move pv_emul_is_mem_write to pv/emulate.c

2017-08-18 Thread Andrew Cooper
On 18/08/17 13:08, Wei Liu wrote:
> On Fri, Aug 18, 2017 at 04:08:44AM -0600, Jan Beulich wrote:
>>
>> If it can't be static anymore, and considering it's just a wrapper
>> around another function call, would there be anything wrong with
>> making it an inline function in the header?
> Yes it can be done:
>
> ---8<---
> From 129ea54249114f97fe66b85672f1710c5f63f604 Mon Sep 17 00:00:00 2001
> From: Wei Liu 
> Date: Wed, 19 Jul 2017 16:15:48 +0100
> Subject: [PATCH] x86: move pv_emul_is_mem_write to pv/emulate.h
>
> Make it a static inline function in pv/emulate.h.  In the mean time it
> is required to include pv/emulate.h in x86/mm.c.
>
> The said function will be used later by different emulation handlers
> in later patches.
>
> Signed-off-by: Wei Liu 

Reviewed-by: Andrew Cooper 

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


Re: [Xen-devel] [PATCH v4 06/31] x86: move pv_emul_is_mem_write to pv/emulate.c

2017-08-18 Thread Wei Liu
On Fri, Aug 18, 2017 at 04:08:44AM -0600, Jan Beulich wrote:
> >>> On 17.08.17 at 16:44,  wrote:
> > @@ -5138,13 +5140,6 @@ static int ptwr_emulated_cmpxchg(
> >  container_of(ctxt, struct ptwr_emulate_ctxt, ctxt));
> >  }
> >  
> > -static int pv_emul_is_mem_write(const struct x86_emulate_state *state,
> > -struct x86_emulate_ctxt *ctxt)
> > -{
> > -return x86_insn_is_mem_write(state, ctxt) ? X86EMUL_OKAY
> > -  : X86EMUL_UNHANDLEABLE;
> > -}
> 
> If it can't be static anymore, and considering it's just a wrapper
> around another function call, would there be anything wrong with
> making it an inline function in the header?

Yes it can be done:

---8<---
From 129ea54249114f97fe66b85672f1710c5f63f604 Mon Sep 17 00:00:00 2001
From: Wei Liu 
Date: Wed, 19 Jul 2017 16:15:48 +0100
Subject: [PATCH] x86: move pv_emul_is_mem_write to pv/emulate.h

Make it a static inline function in pv/emulate.h.  In the mean time it
is required to include pv/emulate.h in x86/mm.c.

The said function will be used later by different emulation handlers
in later patches.

Signed-off-by: Wei Liu 
---
 xen/arch/x86/mm.c | 9 ++---
 xen/arch/x86/pv/emulate.h | 9 +
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index 5983a56811..e0e655ac31 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -126,6 +126,8 @@
 #include 
 #include 
 
+#include "pv/emulate.h"
+
 /* Mapping of the fixmap space needed early. */
 l1_pgentry_t __section(".bss.page_aligned") __aligned(PAGE_SIZE)
 l1_fixmap[L1_PAGETABLE_ENTRIES];
@@ -5138,13 +5140,6 @@ static int ptwr_emulated_cmpxchg(
 container_of(ctxt, struct ptwr_emulate_ctxt, ctxt));
 }
 
-static int pv_emul_is_mem_write(const struct x86_emulate_state *state,
-struct x86_emulate_ctxt *ctxt)
-{
-return x86_insn_is_mem_write(state, ctxt) ? X86EMUL_OKAY
-  : X86EMUL_UNHANDLEABLE;
-}
-
 static const struct x86_emulate_ops ptwr_emulate_ops = {
 .read   = ptwr_emulated_read,
 .insn_fetch = ptwr_emulated_read,
diff --git a/xen/arch/x86/pv/emulate.h b/xen/arch/x86/pv/emulate.h
index b2b1192d48..656c12f62d 100644
--- a/xen/arch/x86/pv/emulate.h
+++ b/xen/arch/x86/pv/emulate.h
@@ -1,10 +1,19 @@
 #ifndef __PV_EMULATE_H__
 #define __PV_EMULATE_H__
 
+#include 
+
 int pv_emul_read_descriptor(unsigned int sel, const struct vcpu *v,
 unsigned long *base, unsigned long *limit,
 unsigned int *ar, bool insn_fetch);
 
 void pv_emul_instruction_done(struct cpu_user_regs *regs, unsigned long rip);
 
+static inline int pv_emul_is_mem_write(const struct x86_emulate_state *state,
+   struct x86_emulate_ctxt *ctxt)
+{
+return x86_insn_is_mem_write(state, ctxt) ? X86EMUL_OKAY
+  : X86EMUL_UNHANDLEABLE;
+}
+
 #endif /* __PV_EMULATE_H__ */
-- 
2.11.0


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


Re: [Xen-devel] [PATCH v4 06/31] x86: move pv_emul_is_mem_write to pv/emulate.c

2017-08-18 Thread Jan Beulich
>>> On 17.08.17 at 16:44,  wrote:
> @@ -5138,13 +5140,6 @@ static int ptwr_emulated_cmpxchg(
>  container_of(ctxt, struct ptwr_emulate_ctxt, ctxt));
>  }
>  
> -static int pv_emul_is_mem_write(const struct x86_emulate_state *state,
> -struct x86_emulate_ctxt *ctxt)
> -{
> -return x86_insn_is_mem_write(state, ctxt) ? X86EMUL_OKAY
> -  : X86EMUL_UNHANDLEABLE;
> -}

If it can't be static anymore, and considering it's just a wrapper
around another function call, would there be anything wrong with
making it an inline function in the header?

Jan


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


Re: [Xen-devel] [PATCH v4 06/31] x86: move pv_emul_is_mem_write to pv/emulate.c

2017-08-17 Thread Andrew Cooper
On 17/08/17 15:44, Wei Liu wrote:
> Export it via pv/emulate.h.  In the mean time it is required to
> include pv/emulate.h in x86/mm.c.
>
> The said function will be used later by different emulation handlers
> in later patches.
>
> Signed-off-by: Wei Liu 

Acked-by: Andrew Cooper 

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