[v22,13/17] i386: split svm_helper into sysemu and stub-only user

Commit Message

Claudio Fontana Feb. 24, 2021, 1:34 p.m. UTC

For now we just copy over the previous user stubs, but really,

everything that requires s->cpl == 0 should be impossible
to trigger from user-mode emulation.

Later on we should add a check that asserts this easily f.e.:

static bool check_cpl0(DisasContext *s)
{
     int cpl = s->cpl;
 #ifdef CONFIG_USER_ONLY
     assert(cpl == 3);
 #endif
     if (cpl != 0) {
         gen_exception(s, EXCP0D_GPF, s->pc_start - s->cs_base);
         return false;
     }
     return true;
}

Signed-off-by: Claudio Fontana <[e-mail]>
Cc: Paolo Bonzini <[e-mail]>
---
 target/i386/tcg/{ => sysemu}/svm_helper.c | 62 +-----------------
 target/i386/tcg/user/svm_stubs.c          | 76 +++++++++++++++++++++++
 target/i386/tcg/meson.build               |  1 -
 target/i386/tcg/sysemu/meson.build        |  1 +
 target/i386/tcg/user/meson.build          |  1 +
 5 files changed, 80 insertions(+), 61 deletions(-)
 rename target/i386/tcg/{ => sysemu}/svm_helper.c (96%)
 create mode 100644 target/i386/tcg/user/svm_stubs.c

Comments

Richard Henderson Feb. 25, 2021, 3:37 a.m. UTC | #1

On 2/24/21 5:34 AM, Claudio Fontana wrote:
> For now we just copy over the previous user stubs, but really,
> 
> everything that requires s->cpl == 0 should be impossible
> to trigger from user-mode emulation.
> 
> Later on we should add a check that asserts this easily f.e.:
> 
> static bool check_cpl0(DisasContext *s)
> {
>      int cpl = s->cpl;
>  #ifdef CONFIG_USER_ONLY
>      assert(cpl == 3);
>  #endif
>      if (cpl != 0) {
>          gen_exception(s, EXCP0D_GPF, s->pc_start - s->cs_base);
>          return false;
>      }
>      return true;
> }

Hmm, more or less exactly what I was suggesting vs patch 11, with the
improvement to consolidate the generation of the exception.

Is it worth doing this now rather than later, so that we don't have to have the
stubs, and so don't have to add the new file?

If you were adding the stubs new, rather than moving them, I'd say add
g_assert_not_reached to them.  But the actual patch is fine.

Reviewed-by: Richard Henderson <[e-mail]>


r~

Patch

diff --git a/target/i386/tcg/svm_helper.c b/target/i386/tcg/sysemu/svm_helper.c
similarity index 96%
rename from target/i386/tcg/svm_helper.c
rename to target/i386/tcg/sysemu/svm_helper.c
index 097bb9b83d..5b9c6f18be 100644
--- a/target/i386/tcg/svm_helper.c
+++ b/target/i386/tcg/sysemu/svm_helper.c
@@ -1,5 +1,5 @@ 
 /*
- *  x86 SVM helpers
+ *  x86 SVM helpers (sysemu only)
  *
  *  Copyright (c) 2003 Fabrice Bellard
  *
@@ -22,66 +22,10 @@ 
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
-#include "helper-tcg.h"
+#include "tcg/helper-tcg.h"
 
 /* Secure Virtual Machine helpers */
 
-#if defined(CONFIG_USER_ONLY)
-
-void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
-{
-}
-
-void helper_vmmcall(CPUX86State *env)
-{
-}
-
-void helper_vmload(CPUX86State *env, int aflag)
-{
-}
-
-void helper_vmsave(CPUX86State *env, int aflag)
-{
-}
-
-void helper_stgi(CPUX86State *env)
-{
-}
-
-void helper_clgi(CPUX86State *env)
-{
-}
-
-void helper_skinit(CPUX86State *env)
-{
-}
-
-void helper_invlpga(CPUX86State *env, int aflag)
-{
-}
-
-void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
-                uintptr_t retaddr)
-{
-    assert(0);
-}
-
-void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
-                                      uint64_t param)
-{
-}
-
-void cpu_svm_check_intercept_param(CPUX86State *env, uint32_t type,
-                                   uint64_t param, uintptr_t retaddr)
-{
-}
-
-void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
-                         uint32_t next_eip_addend)
-{
-}
-#else
-
 static inline void svm_save_seg(CPUX86State *env, hwaddr addr,
                                 const SegmentCache *sc)
 {
@@ -797,5 +741,3 @@  void do_vmexit(CPUX86State *env, uint32_t exit_code, uint64_t exit_info_1)
        host's code segment or non-canonical (in the case of long mode), a
        #GP fault is delivered inside the host. */
 }
-
-#endif
diff --git a/target/i386/tcg/user/svm_stubs.c b/target/i386/tcg/user/svm_stubs.c
new file mode 100644
index 0000000000..97528b56ad
--- /dev/null
+++ b/target/i386/tcg/user/svm_stubs.c
@@ -0,0 +1,76 @@ 
+/*
+ *  x86 SVM helpers (user-mode)
+ *
+ *  Copyright (c) 2003 Fabrice Bellard
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "qemu/osdep.h"
+#include "cpu.h"
+#include "exec/helper-proto.h"
+#include "tcg/helper-tcg.h"
+
+void helper_vmrun(CPUX86State *env, int aflag, int next_eip_addend)
+{
+}
+
+void helper_vmmcall(CPUX86State *env)
+{
+}
+
+void helper_vmload(CPUX86State *env, int aflag)
+{
+}
+
+void helper_vmsave(CPUX86State *env, int aflag)
+{
+}
+
+void helper_stgi(CPUX86State *env)
+{
+}
+
+void helper_clgi(CPUX86State *env)
+{
+}
+
+void helper_skinit(CPUX86State *env)
+{
+}
+
+void helper_invlpga(CPUX86State *env, int aflag)
+{
+}
+
+void cpu_vmexit(CPUX86State *nenv, uint32_t exit_code, uint64_t exit_info_1,
+                uintptr_t retaddr)
+{
+    assert(0);
+}
+
+void helper_svm_check_intercept_param(CPUX86State *env, uint32_t type,
+                                      uint64_t param)
+{
+}
+
+void cpu_svm_check_intercept_param(CPUX86State *env, uint32_t type,
+                                   uint64_t param, uintptr_t retaddr)
+{
+}
+
+void helper_svm_check_io(CPUX86State *env, uint32_t port, uint32_t param,
+                         uint32_t next_eip_addend)
+{
+}
diff --git a/target/i386/tcg/meson.build b/target/i386/tcg/meson.build
index 449d9719ef..f9110e890c 100644
--- a/target/i386/tcg/meson.build
+++ b/target/i386/tcg/meson.build
@@ -8,7 +8,6 @@  i386_ss.add(when: 'CONFIG_TCG', if_true: files(
   'misc_helper.c',
   'mpx_helper.c',
   'seg_helper.c',
-  'svm_helper.c',
   'tcg-cpu.c',
   'translate.c'), if_false: files('tcg-stub.c'))
 
diff --git a/target/i386/tcg/sysemu/meson.build b/target/i386/tcg/sysemu/meson.build
index f84519a213..126528d0c9 100644
--- a/target/i386/tcg/sysemu/meson.build
+++ b/target/i386/tcg/sysemu/meson.build
@@ -5,4 +5,5 @@  i386_softmmu_ss.add(when: ['CONFIG_TCG', 'CONFIG_SOFTMMU'], if_true: files(
   'bpt_helper.c',
   'misc_helper.c',
   'fpu_helper.c',
+  'svm_helper.c',
 ))
diff --git a/target/i386/tcg/user/meson.build b/target/i386/tcg/user/meson.build
index 30eec3f5a4..c540ca2174 100644
--- a/target/i386/tcg/user/meson.build
+++ b/target/i386/tcg/user/meson.build
@@ -2,4 +2,5 @@  i386_user_ss.add(when: ['CONFIG_TCG', 'CONFIG_USER_ONLY'], if_true: files(
   'excp_helper.c',
   'misc_helper.c',
   'fpu_helper.c',
+  'svm_stubs.c',
 ))