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

[xen master] x86emul: drop "seg" parameter from insn_fetch() hook



commit 7b99e7258559c9caa235d9faf323b22c68e4a581
Author:     Jan Beulich <jbeulich@xxxxxxxx>
AuthorDate: Tue Dec 14 09:48:17 2021 +0100
Commit:     Jan Beulich <jbeulich@xxxxxxxx>
CommitDate: Tue Dec 14 09:48:17 2021 +0100

    x86emul: drop "seg" parameter from insn_fetch() hook
    
    This is specified (and asserted for in a number of places) to always be
    CS. Passing this as an argument in various places is therefore
    pointless. The price to pay is two simple new functions, with the
    benefit of the PTWR case now gaining a more appropriate error code.
    
    Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
    Reviewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
    Acked-by: Paul Durrant <paul@xxxxxxx>
---
 tools/fuzz/x86_instruction_emulator/fuzz-emul.c |  5 +----
 tools/tests/x86_emulator/predicates.c           |  6 ++----
 tools/tests/x86_emulator/test_x86_emulator.c    |  3 +--
 tools/tests/x86_emulator/x86-emulate.h          |  3 +--
 xen/arch/x86/hvm/emulate.c                      |  3 +--
 xen/arch/x86/mm/shadow/hvm.c                    |  7 ++-----
 xen/arch/x86/pv/emul-gate-op.c                  |  8 +++++++-
 xen/arch/x86/pv/emul-priv-op.c                  |  5 +----
 xen/arch/x86/pv/ro-page-fault.c                 | 21 ++++++++++++++++++---
 xen/arch/x86/x86_emulate/x86_emulate.c          | 13 ++++++-------
 xen/arch/x86/x86_emulate/x86_emulate.h          |  8 +++-----
 xen/include/asm-x86/hvm/emulate.h               |  3 +--
 12 files changed, 44 insertions(+), 41 deletions(-)

diff --git a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c 
b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
index 28285aad24..966e46bee1 100644
--- a/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
+++ b/tools/fuzz/x86_instruction_emulator/fuzz-emul.c
@@ -197,14 +197,11 @@ static int fuzz_read_io(
 }
 
 static int fuzz_insn_fetch(
-    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
-    assert(seg == x86_seg_cs);
-
     /* Minimal segment limit checking, until full one is being put in place. */
     if ( ctxt->addr_size < 64 && (offset >> 32) )
     {
@@ -222,7 +219,7 @@ static int fuzz_insn_fetch(
         return maybe_fail(ctxt, "insn_fetch", true);
     }
 
-    return data_read(ctxt, seg, "insn_fetch", p_data, bytes);
+    return data_read(ctxt, x86_seg_cs, "insn_fetch", p_data, bytes);
 }
 
 static int _fuzz_rep_read(struct x86_emulate_ctxt *ctxt,
diff --git a/tools/tests/x86_emulator/predicates.c 
b/tools/tests/x86_emulator/predicates.c
index 94b99c94e8..4760f19bf2 100644
--- a/tools/tests/x86_emulator/predicates.c
+++ b/tools/tests/x86_emulator/predicates.c
@@ -2049,8 +2049,7 @@ static void print_insn(const uint8_t *instr, unsigned int 
len)
 
 void do_test(uint8_t *instr, unsigned int len, unsigned int modrm,
              enum mem_access mem, struct x86_emulate_ctxt *ctxt,
-             int (*fetch)(enum x86_segment seg,
-                          unsigned long offset,
+             int (*fetch)(unsigned long offset,
                           void *p_data,
                           unsigned int bytes,
                           struct x86_emulate_ctxt *ctxt))
@@ -2110,8 +2109,7 @@ void do_test(uint8_t *instr, unsigned int len, unsigned 
int modrm,
 }
 
 void predicates_test(void *instr, struct x86_emulate_ctxt *ctxt,
-                     int (*fetch)(enum x86_segment seg,
-                                  unsigned long offset,
+                     int (*fetch)(unsigned long offset,
                                   void *p_data,
                                   unsigned int bytes,
                                   struct x86_emulate_ctxt *ctxt))
diff --git a/tools/tests/x86_emulator/test_x86_emulator.c 
b/tools/tests/x86_emulator/test_x86_emulator.c
index 9b3e98a1a5..ac250c11c4 100644
--- a/tools/tests/x86_emulator/test_x86_emulator.c
+++ b/tools/tests/x86_emulator/test_x86_emulator.c
@@ -594,14 +594,13 @@ static int read(
 }
 
 static int fetch(
-    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
     struct x86_emulate_ctxt *ctxt)
 {
     if ( verbose )
-        printf("** %s(%u, %p,, %u,)\n", __func__, seg, (void *)offset, bytes);
+        printf("** %s(CS:%p,, %u,)\n", __func__, (void *)offset, bytes);
 
     memcpy(p_data, (void *)offset, bytes);
     return X86EMUL_OKAY;
diff --git a/tools/tests/x86_emulator/x86-emulate.h 
b/tools/tests/x86_emulator/x86-emulate.h
index e1a2aaef68..7f60ef9e89 100644
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -105,8 +105,7 @@ WRAP(puts);
 void evex_disp8_test(void *instr, struct x86_emulate_ctxt *ctxt,
                      const struct x86_emulate_ops *ops);
 void predicates_test(void *instr, struct x86_emulate_ctxt *ctxt,
-                     int (*fetch)(enum x86_segment seg,
-                                  unsigned long offset,
+                     int (*fetch)(unsigned long offset,
                                   void *p_data,
                                   unsigned int bytes,
                                   struct x86_emulate_ctxt *ctxt));
diff --git a/xen/arch/x86/hvm/emulate.c b/xen/arch/x86/hvm/emulate.c
index 76a2ccfafe..dd59ad89be 100644
--- a/xen/arch/x86/hvm/emulate.c
+++ b/xen/arch/x86/hvm/emulate.c
@@ -1294,7 +1294,6 @@ static int hvmemul_read(
 }
 
 int hvmemul_insn_fetch(
-    enum x86_segment seg,
     unsigned long offset,
     void *p_data,
     unsigned int bytes,
@@ -1312,7 +1311,7 @@ int hvmemul_insn_fetch(
     if ( !bytes ||
          unlikely((insn_off + bytes) > hvmemul_ctxt->insn_buf_bytes) )
     {
-        int rc = __hvmemul_read(seg, offset, p_data, bytes,
+        int rc = __hvmemul_read(x86_seg_cs, offset, p_data, bytes,
                                 hvm_access_insn_fetch, hvmemul_ctxt);
 
         if ( rc == X86EMUL_OKAY && bytes )
diff --git a/xen/arch/x86/mm/shadow/hvm.c b/xen/arch/x86/mm/shadow/hvm.c
index d5f42102a0..f2991bc176 100644
--- a/xen/arch/x86/mm/shadow/hvm.c
+++ b/xen/arch/x86/mm/shadow/hvm.c
@@ -162,8 +162,7 @@ hvm_emulate_read(enum x86_segment seg,
 }
 
 static int
-hvm_emulate_insn_fetch(enum x86_segment seg,
-                       unsigned long offset,
+hvm_emulate_insn_fetch(unsigned long offset,
                        void *p_data,
                        unsigned int bytes,
                        struct x86_emulate_ctxt *ctxt)
@@ -172,11 +171,9 @@ hvm_emulate_insn_fetch(enum x86_segment seg,
         container_of(ctxt, struct sh_emulate_ctxt, ctxt);
     unsigned int insn_off = offset - sh_ctxt->insn_buf_eip;
 
-    ASSERT(seg == x86_seg_cs);
-
     /* Fall back if requested bytes are not in the prefetch cache. */
     if ( unlikely((insn_off + bytes) > sh_ctxt->insn_buf_bytes) )
-        return hvm_read(seg, offset, p_data, bytes,
+        return hvm_read(x86_seg_cs, offset, p_data, bytes,
                         hvm_access_insn_fetch, sh_ctxt);
 
     /* Hit the cache. Simple memcpy. */
diff --git a/xen/arch/x86/pv/emul-gate-op.c b/xen/arch/x86/pv/emul-gate-op.c
index 43d847abd0..68ec4d11f6 100644
--- a/xen/arch/x86/pv/emul-gate-op.c
+++ b/xen/arch/x86/pv/emul-gate-op.c
@@ -163,6 +163,12 @@ static int read_mem(enum x86_segment seg, unsigned long 
offset, void *p_data,
     return X86EMUL_OKAY;
 }
 
+static int fetch(unsigned long offset, void *p_data,
+                 unsigned int bytes, struct x86_emulate_ctxt *ctxt)
+{
+    return read_mem(x86_seg_cs, offset, p_data, bytes, ctxt);
+}
+
 void pv_emulate_gate_op(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
@@ -205,7 +211,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, read_mem);
+    state = x86_decode_insn(&ctxt.ctxt, fetch);
     ctxt.insn_fetch = false;
     if ( IS_ERR_OR_NULL(state) )
     {
diff --git a/xen/arch/x86/pv/emul-priv-op.c b/xen/arch/x86/pv/emul-priv-op.c
index 8ba65178e9..c78be6d92b 100644
--- a/xen/arch/x86/pv/emul-priv-op.c
+++ b/xen/arch/x86/pv/emul-priv-op.c
@@ -1258,8 +1258,7 @@ static int validate(const struct x86_emulate_state *state,
     return X86EMUL_UNHANDLEABLE;
 }
 
-static int insn_fetch(enum x86_segment seg,
-                      unsigned long offset,
+static int insn_fetch(unsigned long offset,
                       void *p_data,
                       unsigned int bytes,
                       struct x86_emulate_ctxt *ctxt)
@@ -1269,8 +1268,6 @@ static int insn_fetch(enum x86_segment seg,
     unsigned int rc;
     unsigned long addr = poc->cs.base + offset;
 
-    ASSERT(seg == x86_seg_cs);
-
     /* We don't mean to emulate any branches. */
     if ( !bytes )
         return X86EMUL_UNHANDLEABLE;
diff --git a/xen/arch/x86/pv/ro-page-fault.c b/xen/arch/x86/pv/ro-page-fault.c
index ac5b66870c..ef4d146c1d 100644
--- a/xen/arch/x86/pv/ro-page-fault.c
+++ b/xen/arch/x86/pv/ro-page-fault.c
@@ -52,6 +52,21 @@ static int ptwr_emulated_read(enum x86_segment seg, unsigned 
long offset,
     return X86EMUL_OKAY;
 }
 
+static int ptwr_emulated_insn_fetch(unsigned long offset,
+                                    void *p_data, unsigned int bytes,
+                                    struct x86_emulate_ctxt *ctxt)
+{
+    unsigned int rc = copy_from_guest_pv(p_data, (void *)offset, bytes);
+
+    if ( rc )
+    {
+        x86_emul_pagefault(PFEC_insn_fetch, offset + bytes - rc, ctxt);
+        return X86EMUL_EXCEPTION;
+    }
+
+    return X86EMUL_OKAY;
+}
+
 /*
  * p_old being NULL indicates a plain write to occur, while a non-NULL
  * input requests a CMPXCHG-based update.
@@ -247,7 +262,7 @@ static int ptwr_emulated_cmpxchg(enum x86_segment seg, 
unsigned long offset,
 
 static const struct x86_emulate_ops ptwr_emulate_ops = {
     .read       = ptwr_emulated_read,
-    .insn_fetch = ptwr_emulated_read,
+    .insn_fetch = ptwr_emulated_insn_fetch,
     .write      = ptwr_emulated_write,
     .cmpxchg    = ptwr_emulated_cmpxchg,
     .validate   = pv_emul_is_mem_write,
@@ -290,14 +305,14 @@ static int ptwr_do_page_fault(struct x86_emulate_ctxt 
*ctxt,
 
 static const struct x86_emulate_ops mmio_ro_emulate_ops = {
     .read       = x86emul_unhandleable_rw,
-    .insn_fetch = ptwr_emulated_read,
+    .insn_fetch = ptwr_emulated_insn_fetch,
     .write      = mmio_ro_emulated_write,
     .validate   = pv_emul_is_mem_write,
 };
 
 static const struct x86_emulate_ops mmcfg_intercept_ops = {
     .read       = x86emul_unhandleable_rw,
-    .insn_fetch = ptwr_emulated_read,
+    .insn_fetch = ptwr_emulated_insn_fetch,
     .write      = mmcfg_intercept_write,
     .validate   = pv_emul_is_mem_write,
 };
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.c 
b/xen/arch/x86/x86_emulate/x86_emulate.c
index 069acde517..5b43bda239 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.c
+++ b/xen/arch/x86/x86_emulate/x86_emulate.c
@@ -1295,7 +1295,7 @@ static inline int mkec(uint8_t e, int32_t ec, ...)
    generate_exception_if((uint8_t)(state->ip -                          \
                                    ctxt->regs->r(ip)) > MAX_INST_LEN,   \
                          EXC_GP, 0);                                    \
-   rc = ops->insn_fetch(x86_seg_cs, _ip, &_x, (_size), ctxt);           \
+   rc = ops->insn_fetch(_ip, &_x, _size, ctxt);                         \
    if ( rc ) goto done;                                                 \
    _x;                                                                  \
 })
@@ -1363,7 +1363,7 @@ do {                                                      
              \
         ip = (uint16_t)ip;                                              \
     else if ( !mode_64bit() )                                           \
         ip = (uint32_t)ip;                                              \
-    rc = ops->insn_fetch(x86_seg_cs, ip, NULL, 0, ctxt);                \
+    rc = ops->insn_fetch(ip, NULL, 0, ctxt);                            \
     if ( rc ) goto done;                                                \
     _regs.r(ip) = ip;                                                   \
     singlestep = _regs.eflags & X86_EFLAGS_TF;                          \
@@ -4740,7 +4740,7 @@ x86_emulate(
                    ? 8 : op_bytes;
         if ( (rc = read_ulong(x86_seg_ss, sp_post_inc(op_bytes + src.val),
                               &dst.val, op_bytes, ctxt, ops)) != 0 ||
-             (rc = ops->insn_fetch(x86_seg_cs, dst.val, NULL, 0, ctxt)) )
+             (rc = ops->insn_fetch(dst.val, NULL, 0, ctxt)) )
             goto done;
         _regs.r(ip) = dst.val;
         adjust_bnd(ctxt, ops, vex.pfx);
@@ -5628,14 +5628,14 @@ x86_emulate(
             break;
         case 2: /* call (near) */
             dst.val = _regs.r(ip);
-            if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) )
+            if ( (rc = ops->insn_fetch(src.val, NULL, 0, ctxt)) )
                 goto done;
             _regs.r(ip) = src.val;
             src.val = dst.val;
             adjust_bnd(ctxt, ops, vex.pfx);
             goto push;
         case 4: /* jmp (near) */
-            if ( (rc = ops->insn_fetch(x86_seg_cs, src.val, NULL, 0, ctxt)) )
+            if ( (rc = ops->insn_fetch(src.val, NULL, 0, ctxt)) )
                 goto done;
             _regs.r(ip) = src.val;
             dst.type = OP_NONE;
@@ -12220,8 +12220,7 @@ struct x86_emulate_state *
 x86_decode_insn(
     struct x86_emulate_ctxt *ctxt,
     int (*insn_fetch)(
-        enum x86_segment seg, unsigned long offset,
-        void *p_data, unsigned int bytes,
+        unsigned long offset, void *p_data, unsigned int bytes,
         struct x86_emulate_ctxt *ctxt))
 {
     static DEFINE_PER_CPU(struct x86_emulate_state, state);
diff --git a/xen/arch/x86/x86_emulate/x86_emulate.h 
b/xen/arch/x86/x86_emulate/x86_emulate.h
index d8fb3a9909..419def8790 100644
--- a/xen/arch/x86/x86_emulate/x86_emulate.h
+++ b/xen/arch/x86/x86_emulate/x86_emulate.h
@@ -254,13 +254,12 @@ struct x86_emulate_ops
 
     /*
      * insn_fetch: Emulate fetch from instruction byte stream.
-     *  Except for @bytes, all parameters are the same as for 'read'.
+     *  Except for @bytes and missing @seg, all parameters are the same as for
+     *  'read'.
      *  @bytes: Access length (0 <= @bytes < 16, with zero meaning
      *  "validate address only").
-     *  @seg is always x86_seg_cs.
      */
     int (*insn_fetch)(
-        enum x86_segment seg,
         unsigned long offset,
         void *p_data,
         unsigned int bytes,
@@ -750,8 +749,7 @@ struct x86_emulate_state *
 x86_decode_insn(
     struct x86_emulate_ctxt *ctxt,
     int (*insn_fetch)(
-        enum x86_segment seg, unsigned long offset,
-        void *p_data, unsigned int bytes,
+        unsigned long offset, void *p_data, unsigned int bytes,
         struct x86_emulate_ctxt *ctxt));
 
 unsigned int
diff --git a/xen/include/asm-x86/hvm/emulate.h 
b/xen/include/asm-x86/hvm/emulate.h
index 610078b28f..e670040603 100644
--- a/xen/include/asm-x86/hvm/emulate.h
+++ b/xen/include/asm-x86/hvm/emulate.h
@@ -92,8 +92,7 @@ static inline bool handle_mmio(void)
     return hvm_emulate_one_insn(x86_insn_is_mem_access, "MMIO");
 }
 
-int hvmemul_insn_fetch(enum x86_segment seg,
-                       unsigned long offset,
+int hvmemul_insn_fetch(unsigned long offset,
                        void *p_data,
                        unsigned int bytes,
                        struct x86_emulate_ctxt *ctxt);
--
generated by git-patchbot for /home/xen/git/xen.git#master



 


Rackspace

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