[PATCH 0/2] x86/pv: drop unnecessary prefixes

xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed

* [PATCH 0/2] x86/pv: drop unnecessary prefixes
@ 2017-08-31 11:45 Wei Liu
  2017-08-31 11:45 ` [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c Wei Liu
  2017-08-31 11:45 ` [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c Wei Liu
  0 siblings, 2 replies; 7+ messages in thread
From: Wei Liu @ 2017-08-31 11:45 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Wei Liu (2):
  x86/pv: drop priv_op prefix in emul-priv-op.c
  x86/pv: drop gate_op prefix in emul-gate-op.c

 xen/arch/x86/pv/emul-gate-op.c | 15 +++----
 xen/arch/x86/pv/emul-priv-op.c | 99 +++++++++++++++++++++---------------------
 2 files changed, 55 insertions(+), 59 deletions(-)

-- 
2.11.0


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c
  2017-08-31 11:45 [PATCH 0/2] x86/pv: drop unnecessary prefixes Wei Liu
@ 2017-08-31 11:45 ` Wei Liu
  2017-08-31 12:02   ` Jan Beulich
  2017-08-31 11:45 ` [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c Wei Liu
  1 sibling, 1 reply; 7+ messages in thread
From: Wei Liu @ 2017-08-31 11:45 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

Drop the prefix because they live in their own file now. One exception
is wbinvd handler because drpooing the prefix will clash with the
actual wbinvd function.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/emul-priv-op.c | 99 +++++++++++++++++++++---------------------
 1 file changed, 50 insertions(+), 49 deletions(-)

diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 54a63c27f8..1f3e24b169 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -316,8 +316,8 @@ static unsigned int check_guest_io_breakpoint(struct vcpu *v,
     return match;
 }
 
-static int priv_op_read_io(unsigned int port, unsigned int bytes,
-                           unsigned long *val, struct x86_emulate_ctxt *ctxt)
+static int read_io(unsigned int port, unsigned int bytes,
+                   unsigned long *val, struct x86_emulate_ctxt *ctxt)
 {
     struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
     struct vcpu *curr = current;
@@ -415,8 +415,8 @@ void guest_io_write(unsigned int port, unsigned int bytes, uint32_t data,
     }
 }
 
-static int priv_op_write_io(unsigned int port, unsigned int bytes,
-                            unsigned long val, struct x86_emulate_ctxt *ctxt)
+static int write_io(unsigned int port, unsigned int bytes,
+                    unsigned long val, struct x86_emulate_ctxt *ctxt)
 {
     struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
     struct vcpu *curr = current;
@@ -447,9 +447,9 @@ static int priv_op_write_io(unsigned int port, unsigned int bytes,
     return X86EMUL_OKAY;
 }
 
-static int priv_op_read_segment(enum x86_segment seg,
-                                struct segment_register *reg,
-                                struct x86_emulate_ctxt *ctxt)
+static int read_segment(enum x86_segment seg,
+                        struct segment_register *reg,
+                        struct x86_emulate_ctxt *ctxt)
 {
     /* Check if this is an attempt to access the I/O bitmap. */
     if ( seg == x86_seg_tr )
@@ -561,10 +561,10 @@ static int pv_emul_virt_to_linear(unsigned long base, unsigned long offset,
     return rc;
 }
 
-static int priv_op_rep_ins(uint16_t port,
-                           enum x86_segment seg, unsigned long offset,
-                           unsigned int bytes_per_rep, unsigned long *reps,
-                           struct x86_emulate_ctxt *ctxt)
+static int rep_ins(uint16_t port,
+                   enum x86_segment seg, unsigned long offset,
+                   unsigned int bytes_per_rep, unsigned long *reps,
+                   struct x86_emulate_ctxt *ctxt)
 {
     struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
     struct vcpu *curr = current;
@@ -580,7 +580,7 @@ static int priv_op_rep_ins(uint16_t port,
     if ( !guest_io_okay(port, bytes_per_rep, curr, ctxt->regs) )
         return X86EMUL_UNHANDLEABLE;
 
-    rc = priv_op_read_segment(x86_seg_es, &sreg, ctxt);
+    rc = read_segment(x86_seg_es, &sreg, ctxt);
     if ( rc != X86EMUL_OKAY )
         return rc;
 
@@ -628,10 +628,10 @@ static int priv_op_rep_ins(uint16_t port,
     return X86EMUL_OKAY;
 }
 
-static int priv_op_rep_outs(enum x86_segment seg, unsigned long offset,
-                            uint16_t port,
-                            unsigned int bytes_per_rep, unsigned long *reps,
-                            struct x86_emulate_ctxt *ctxt)
+static int rep_outs(enum x86_segment seg, unsigned long offset,
+                    uint16_t port,
+                    unsigned int bytes_per_rep, unsigned long *reps,
+                    struct x86_emulate_ctxt *ctxt)
 {
     struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
     struct vcpu *curr = current;
@@ -645,7 +645,7 @@ static int priv_op_rep_outs(enum x86_segment seg, unsigned long offset,
     if ( !guest_io_okay(port, bytes_per_rep, curr, ctxt->regs) )
         return X86EMUL_UNHANDLEABLE;
 
-    rc = priv_op_read_segment(seg, &sreg, ctxt);
+    rc = read_segment(seg, &sreg, ctxt);
     if ( rc != X86EMUL_OKAY )
         return rc;
 
@@ -696,8 +696,8 @@ static int priv_op_rep_outs(enum x86_segment seg, unsigned long offset,
     return X86EMUL_OKAY;
 }
 
-static int priv_op_read_cr(unsigned int reg, unsigned long *val,
-                           struct x86_emulate_ctxt *ctxt)
+static int read_cr(unsigned int reg, unsigned long *val,
+                   struct x86_emulate_ctxt *ctxt)
 {
     const struct vcpu *curr = current;
 
@@ -740,8 +740,8 @@ static int priv_op_read_cr(unsigned int reg, unsigned long *val,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int priv_op_write_cr(unsigned int reg, unsigned long val,
-                            struct x86_emulate_ctxt *ctxt)
+static int write_cr(unsigned int reg, unsigned long val,
+                    struct x86_emulate_ctxt *ctxt)
 {
     struct vcpu *curr = current;
 
@@ -797,8 +797,8 @@ static int priv_op_write_cr(unsigned int reg, unsigned long val,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int priv_op_read_dr(unsigned int reg, unsigned long *val,
-                           struct x86_emulate_ctxt *ctxt)
+static int read_dr(unsigned int reg, unsigned long *val,
+                   struct x86_emulate_ctxt *ctxt)
 {
     unsigned long res = do_get_debugreg(reg);
 
@@ -810,8 +810,8 @@ static int priv_op_read_dr(unsigned int reg, unsigned long *val,
     return X86EMUL_OKAY;
 }
 
-static int priv_op_write_dr(unsigned int reg, unsigned long val,
-                            struct x86_emulate_ctxt *ctxt)
+static int write_dr(unsigned int reg, unsigned long val,
+                    struct x86_emulate_ctxt *ctxt)
 {
     return do_set_debugreg(reg, val) == 0
            ? X86EMUL_OKAY : X86EMUL_UNHANDLEABLE;
@@ -833,8 +833,8 @@ static inline bool is_cpufreq_controller(const struct domain *d)
             is_hardware_domain(d));
 }
 
-static int priv_op_read_msr(unsigned int reg, uint64_t *val,
-                            struct x86_emulate_ctxt *ctxt)
+static int read_msr(unsigned int reg, uint64_t *val,
+                    struct x86_emulate_ctxt *ctxt)
 {
     struct priv_op_ctxt *poc = container_of(ctxt, struct priv_op_ctxt, ctxt);
     const struct vcpu *curr = current;
@@ -997,8 +997,8 @@ static int priv_op_read_msr(unsigned int reg, uint64_t *val,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int priv_op_write_msr(unsigned int reg, uint64_t val,
-                             struct x86_emulate_ctxt *ctxt)
+static int write_msr(unsigned int reg, uint64_t val,
+                     struct x86_emulate_ctxt *ctxt)
 {
     struct vcpu *curr = current;
     const struct domain *currd = curr->domain;
@@ -1205,6 +1205,7 @@ static int priv_op_write_msr(unsigned int reg, uint64_t val,
     return X86EMUL_UNHANDLEABLE;
 }
 
+/* Name it differently to avoid clashing with wbinvd() */
 static int priv_op_wbinvd(struct x86_emulate_ctxt *ctxt)
 {
     /* Ignore the instruction if unprivileged. */
@@ -1228,8 +1229,8 @@ int pv_emul_cpuid(uint32_t leaf, uint32_t subleaf,
     return X86EMUL_OKAY;
 }
 
-static int priv_op_validate(const struct x86_emulate_state *state,
-                            struct x86_emulate_ctxt *ctxt)
+static int validate(const struct x86_emulate_state *state,
+                    struct x86_emulate_ctxt *ctxt)
 {
     switch ( ctxt->opcode )
     {
@@ -1278,11 +1279,11 @@ static int priv_op_validate(const struct x86_emulate_state *state,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int priv_op_insn_fetch(enum x86_segment seg,
-                              unsigned long offset,
-                              void *p_data,
-                              unsigned int bytes,
-                              struct x86_emulate_ctxt *ctxt)
+static int insn_fetch(enum x86_segment seg,
+                      unsigned long offset,
+                      void *p_data,
+                      unsigned int bytes,
+                      struct x86_emulate_ctxt *ctxt)
 {
     const struct priv_op_ctxt *poc =
         container_of(ctxt, struct priv_op_ctxt, ctxt);
@@ -1316,20 +1317,20 @@ static int priv_op_insn_fetch(enum x86_segment seg,
 
 
 static const struct x86_emulate_ops priv_op_ops = {
-    .insn_fetch          = priv_op_insn_fetch,
+    .insn_fetch          = insn_fetch,
     .read                = x86emul_unhandleable_rw,
-    .validate            = priv_op_validate,
-    .read_io             = priv_op_read_io,
-    .write_io            = priv_op_write_io,
-    .rep_ins             = priv_op_rep_ins,
-    .rep_outs            = priv_op_rep_outs,
-    .read_segment        = priv_op_read_segment,
-    .read_cr             = priv_op_read_cr,
-    .write_cr            = priv_op_write_cr,
-    .read_dr             = priv_op_read_dr,
-    .write_dr            = priv_op_write_dr,
-    .read_msr            = priv_op_read_msr,
-    .write_msr           = priv_op_write_msr,
+    .validate            = validate,
+    .read_io             = read_io,
+    .write_io            = write_io,
+    .rep_ins             = rep_ins,
+    .rep_outs            = rep_outs,
+    .read_segment        = read_segment,
+    .read_cr             = read_cr,
+    .write_cr            = write_cr,
+    .read_dr             = read_dr,
+    .write_dr            = write_dr,
+    .read_msr            = read_msr,
+    .write_msr           = write_msr,
     .cpuid               = pv_emul_cpuid,
     .wbinvd              = priv_op_wbinvd,
 };
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c
  2017-08-31 11:45 ` [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c Wei Liu
@ 2017-08-31 12:02   ` Jan Beulich
  0 siblings, 0 replies; 7+ messages in thread
From: Jan Beulich @ 2017-08-31 12:02 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Xen-devel

>>> On 31.08.17 at 13:45, <wei.liu2@citrix.com> wrote:
> Drop the prefix because they live in their own file now. One exception
> is wbinvd handler because drpooing the prefix will clash with the
> actual wbinvd function.

How about _wbinvd() (single underscores as prefix are okay for
static functions) or do_wbinvd()? With either of these ...

> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

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

... and thanks for doing this!

Jan


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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c
  2017-08-31 11:45 [PATCH 0/2] x86/pv: drop unnecessary prefixes Wei Liu
  2017-08-31 11:45 ` [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c Wei Liu
@ 2017-08-31 11:45 ` Wei Liu
  2017-08-31 12:03   ` Jan Beulich
  1 sibling, 1 reply; 7+ messages in thread
From: Wei Liu @ 2017-08-31 11:45 UTC (permalink / raw)
  To: Xen-devel; +Cc: Andrew Cooper, Wei Liu, Jan Beulich

There is only one function gate_op_read that needs to be modified.

Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
 xen/arch/x86/pv/emul-gate-op.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/xen/arch/x86/pv/emul-gate-op.c b/xen/arch/x86/pv/emul-gate-op.c
index 0a7381a094..002fb782f2 100644
--- a/xen/arch/x86/pv/emul-gate-op.c
+++ b/xen/arch/x86/pv/emul-gate-op.c
@@ -117,12 +117,8 @@ struct gate_op_ctxt {
     bool insn_fetch;
 };
 
-static int gate_op_read(
-    enum x86_segment seg,
-    unsigned long offset,
-    void *p_data,
-    unsigned int bytes,
-    struct x86_emulate_ctxt *ctxt)
+static int read(enum x86_segment seg, unsigned long offset, void *p_data,
+                unsigned int bytes, struct x86_emulate_ctxt *ctxt)
 {
     const struct gate_op_ctxt *goc =
         container_of(ctxt, struct gate_op_ctxt, ctxt);
@@ -230,7 +226,7 @@ void pv_emulate_gate_op(struct cpu_user_regs *regs)
 
     ctxt.ctxt.addr_size = ar & _SEGMENT_DB ? 32 : 16;
     /* Leave zero in ctxt.ctxt.sp_size, as it's not needed for decoding. */
-    state = x86_decode_insn(&ctxt.ctxt, gate_op_read);
+    state = x86_decode_insn(&ctxt.ctxt, read);
     ctxt.insn_fetch = false;
     if ( IS_ERR_OR_NULL(state) )
     {
@@ -265,9 +261,8 @@ void pv_emulate_gate_op(struct cpu_user_regs *regs)
         case 3:
             ++jump;
             base = x86_insn_operand_ea(state, &seg);
-            rc = gate_op_read(seg,
-                              base + (x86_insn_opsize(state) >> 3),
-                              &opnd_sel, sizeof(opnd_sel), &ctxt.ctxt);
+            rc = read(seg, base + (x86_insn_opsize(state) >> 3),
+                      &opnd_sel, sizeof(opnd_sel), &ctxt.ctxt);
             break;
         }
         break;
-- 
2.11.0


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

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c
  2017-08-31 11:45 ` [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c Wei Liu
@ 2017-08-31 12:03   ` Jan Beulich
  2017-08-31 12:36     ` Andrew Cooper
  0 siblings, 1 reply; 7+ messages in thread
From: Jan Beulich @ 2017-08-31 12:03 UTC (permalink / raw)
  To: Wei Liu; +Cc: Andrew Cooper, Xen-devel

>>> On 31.08.17 at 13:45, <wei.liu2@citrix.com> wrote:
> There is only one function gate_op_read that needs to be modified.

I'm fine with it just being read() here, but I can see this being possibly
controversial. Please double check that Andrew isn't entirely opposed
to it. An alternative suggestion would then be read_mem().

> Signed-off-by: Wei Liu <wei.liu2@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] 7+ messages in thread

* Re: [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c
  2017-08-31 12:03   ` Jan Beulich
@ 2017-08-31 12:36     ` Andrew Cooper
  2017-08-31 15:04       ` Wei Liu
  0 siblings, 1 reply; 7+ messages in thread
From: Andrew Cooper @ 2017-08-31 12:36 UTC (permalink / raw)
  To: Jan Beulich, Wei Liu; +Cc: Xen-devel

On 31/08/17 13:03, Jan Beulich wrote:
>>>> On 31.08.17 at 13:45, <wei.liu2@citrix.com> wrote:
>> There is only one function gate_op_read that needs to be modified.
> I'm fine with it just being read() here, but I can see this being possibly
> controversial. Please double check that Andrew isn't entirely opposed
> to it. An alternative suggestion would then be read_mem().

I'm not opposed generally, but simply 'read' has a chance of angering
Coverity, because it has inbuilt models for most functions specified by
core standards like POSIX.

read_mem() would be better.

~Andrew

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c
  2017-08-31 12:36     ` Andrew Cooper
@ 2017-08-31 15:04       ` Wei Liu
  0 siblings, 0 replies; 7+ messages in thread
From: Wei Liu @ 2017-08-31 15:04 UTC (permalink / raw)
  To: Andrew Cooper; +Cc: Xen-devel, Wei Liu, Jan Beulich

On Thu, Aug 31, 2017 at 01:36:13PM +0100, Andrew Cooper wrote:
> On 31/08/17 13:03, Jan Beulich wrote:
> >>>> On 31.08.17 at 13:45, <wei.liu2@citrix.com> wrote:
> >> There is only one function gate_op_read that needs to be modified.
> > I'm fine with it just being read() here, but I can see this being possibly
> > controversial. Please double check that Andrew isn't entirely opposed
> > to it. An alternative suggestion would then be read_mem().
> 
> I'm not opposed generally, but simply 'read' has a chance of angering
> Coverity, because it has inbuilt models for most functions specified by
> core standards like POSIX.
> 
> read_mem() would be better.
> 

read_mem it is.

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-08-31 15:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-31 11:45 [PATCH 0/2] x86/pv: drop unnecessary prefixes Wei Liu
2017-08-31 11:45 ` [PATCH 1/2] x86/pv: drop priv_op prefix in emul-priv-op.c Wei Liu
2017-08-31 12:02   ` Jan Beulich
2017-08-31 11:45 ` [PATCH 2/2] x86/pv: drop gate_op prefix in emul-gate-op.c Wei Liu
2017-08-31 12:03   ` Jan Beulich
2017-08-31 12:36     ` Andrew Cooper
2017-08-31 15:04       ` Wei Liu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).