diff mbox series

[v2,49/50] target/i386: Move helper_check_io to sysemu

Message ID 20210514151342.384376-50-richard.henderson@linaro.org
State New
Headers show
Series target/i386 translate cleanups | expand

Commit Message

Richard Henderson May 14, 2021, 3:13 p.m. UTC
The we never allow i/o from user-only, and the tss check
that helper_check_io does will always fail.  Use an ifdef
within gen_check_io and return false, indicating that an
exception is known to be raised.

Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
 target/i386/helper.h                |  2 +-
 target/i386/tcg/seg_helper.c        | 28 ----------------------------
 target/i386/tcg/sysemu/seg_helper.c | 29 +++++++++++++++++++++++++++++
 target/i386/tcg/translate.c         | 11 +++++++++++
 4 files changed, 41 insertions(+), 29 deletions(-)

-- 
2.25.1

Comments

Richard Henderson May 14, 2021, 5:45 p.m. UTC | #1
On 5/14/21 10:13 AM, Richard Henderson wrote:
> --- a/target/i386/tcg/translate.c

> +++ b/target/i386/tcg/translate.c

> @@ -193,6 +193,7 @@ typedef struct DisasContext {

>       { qemu_build_not_reached(); }

>   

>   #ifdef CONFIG_USER_ONLY

> +STUB_HELPER(check_io, TCGv_env env, TCGv_i32 port, TCGv_i32 size)

>   STUB_HELPER(clgi, TCGv_env env)

>   STUB_HELPER(flush_page, TCGv_env env, TCGv addr)

>   STUB_HELPER(hlt, TCGv_env env, TCGv_i32 pc_ofs)

...
> @@ -681,6 +683,14 @@ static void gen_helper_out_func(MemOp ot, TCGv_i32 v, TCGv_i32 n)

>   static bool gen_check_io(DisasContext *s, MemOp ot, TCGv_i32 port,

>                            uint32_t svm_flags)

>   {

> +#ifdef CONFIG_USER_ONLY

> +    /*

> +     * We do not implement the iopriv(2) syscall, so the TSS check

> +     * will always fail.

> +     */

> +    gen_exception_gpf(s);

> +    return false;

> +#else

>       if (PE(s) && (CPL(s) > IOPL(s) || VM86(s))) {

>           gen_helper_check_io(cpu_env, port, tcg_constant_i32(1 << ot));

>       }

> @@ -699,6 +709,7 @@ static bool gen_check_io(DisasContext *s, MemOp ot, TCGv_i32 port,

>                                   tcg_constant_i32(next_eip - cur_eip));

>       }

>       return true;

> +#endif


This ifdef means the STUB_HELPER above isn't even used.
This is caught by clang as an unused inline function.
Will fix for v3.


r~
Paolo Bonzini May 18, 2021, 10:22 a.m. UTC | #2
On 14/05/21 19:45, Richard Henderson wrote:
> On 5/14/21 10:13 AM, Richard Henderson wrote:

>> --- a/target/i386/tcg/translate.c

>> +++ b/target/i386/tcg/translate.c

>> @@ -193,6 +193,7 @@ typedef struct DisasContext {

>>       { qemu_build_not_reached(); }

>>   #ifdef CONFIG_USER_ONLY

>> +STUB_HELPER(check_io, TCGv_env env, TCGv_i32 port, TCGv_i32 size)

>>   STUB_HELPER(clgi, TCGv_env env)

>>   STUB_HELPER(flush_page, TCGv_env env, TCGv addr)

>>   STUB_HELPER(hlt, TCGv_env env, TCGv_i32 pc_ofs)

> ...

>> @@ -681,6 +683,14 @@ static void gen_helper_out_func(MemOp ot, 

>> TCGv_i32 v, TCGv_i32 n)

>>   static bool gen_check_io(DisasContext *s, MemOp ot, TCGv_i32 port,

>>                            uint32_t svm_flags)

>>   {

>> +#ifdef CONFIG_USER_ONLY

>> +    /*

>> +     * We do not implement the iopriv(2) syscall, so the TSS check

>> +     * will always fail.

>> +     */

>> +    gen_exception_gpf(s);

>> +    return false;

>> +#else

>>       if (PE(s) && (CPL(s) > IOPL(s) || VM86(s))) {

>>           gen_helper_check_io(cpu_env, port, tcg_constant_i32(1 << ot));

>>       }

>> @@ -699,6 +709,7 @@ static bool gen_check_io(DisasContext *s, MemOp 

>> ot, TCGv_i32 port,

>>                                   tcg_constant_i32(next_eip - cur_eip));

>>       }

>>       return true;

>> +#endif

> 

> This ifdef means the STUB_HELPER above isn't even used.

> This is caught by clang as an unused inline function.

> Will fix for v3.


While you're at it it's ioperm, not iopriv.

Paolo
diff mbox series

Patch

diff --git a/target/i386/helper.h b/target/i386/helper.h
index 47d0d67699..3fd0253298 100644
--- a/target/i386/helper.h
+++ b/target/i386/helper.h
@@ -86,7 +86,6 @@  DEF_HELPER_1(rdtsc, void, env)
 DEF_HELPER_1(rdtscp, void, env)
 DEF_HELPER_FLAGS_1(rdpmc, TCG_CALL_NO_WG, noreturn, env)
 
-DEF_HELPER_FLAGS_3(check_io, TCG_CALL_NO_WG, void, env, i32, i32)
 DEF_HELPER_3(outb, void, env, i32, i32)
 DEF_HELPER_2(inb, tl, env, i32)
 DEF_HELPER_3(outw, void, env, i32, i32)
@@ -95,6 +94,7 @@  DEF_HELPER_3(outl, void, env, i32, i32)
 DEF_HELPER_2(inl, tl, env, i32)
 
 #ifndef CONFIG_USER_ONLY
+DEF_HELPER_FLAGS_3(check_io, TCG_CALL_NO_WG, void, env, i32, i32)
 DEF_HELPER_FLAGS_4(bpt_io, TCG_CALL_NO_WG, void, env, i32, i32, tl)
 DEF_HELPER_2(svm_check_intercept, void, env, i32)
 DEF_HELPER_4(svm_check_io, void, env, i32, i32, i32)
diff --git a/target/i386/tcg/seg_helper.c b/target/i386/tcg/seg_helper.c
index 69d6e8f602..2f6cdc8239 100644
--- a/target/i386/tcg/seg_helper.c
+++ b/target/i386/tcg/seg_helper.c
@@ -2416,31 +2416,3 @@  void helper_verw(CPUX86State *env, target_ulong selector1)
     }
     CC_SRC = eflags | CC_Z;
 }
-
-/* check if Port I/O is allowed in TSS */
-void helper_check_io(CPUX86State *env, uint32_t addr, uint32_t size)
-{
-    uintptr_t retaddr = GETPC();
-    uint32_t io_offset, val, mask;
-
-    /* TSS must be a valid 32 bit one */
-    if (!(env->tr.flags & DESC_P_MASK) ||
-        ((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 ||
-        env->tr.limit < 103) {
-        goto fail;
-    }
-    io_offset = cpu_lduw_kernel_ra(env, env->tr.base + 0x66, retaddr);
-    io_offset += (addr >> 3);
-    /* Note: the check needs two bytes */
-    if ((io_offset + 1) > env->tr.limit) {
-        goto fail;
-    }
-    val = cpu_lduw_kernel_ra(env, env->tr.base + io_offset, retaddr);
-    val >>= (addr & 7);
-    mask = (1 << size) - 1;
-    /* all bits must be zero to allow the I/O */
-    if ((val & mask) != 0) {
-    fail:
-        raise_exception_err_ra(env, EXCP0D_GPF, 0, retaddr);
-    }
-}
diff --git a/target/i386/tcg/sysemu/seg_helper.c b/target/i386/tcg/sysemu/seg_helper.c
index e0d7b32b82..82c0856c41 100644
--- a/target/i386/tcg/sysemu/seg_helper.c
+++ b/target/i386/tcg/sysemu/seg_helper.c
@@ -23,6 +23,7 @@ 
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
 #include "tcg/helper-tcg.h"
+#include "../seg_helper.h"
 
 #ifdef TARGET_X86_64
 void helper_syscall(CPUX86State *env, int next_eip_addend)
@@ -123,3 +124,31 @@  void x86_cpu_do_interrupt(CPUState *cs)
         env->old_exception = -1;
     }
 }
+
+/* check if Port I/O is allowed in TSS */
+void helper_check_io(CPUX86State *env, uint32_t addr, uint32_t size)
+{
+    uintptr_t retaddr = GETPC();
+    uint32_t io_offset, val, mask;
+
+    /* TSS must be a valid 32 bit one */
+    if (!(env->tr.flags & DESC_P_MASK) ||
+        ((env->tr.flags >> DESC_TYPE_SHIFT) & 0xf) != 9 ||
+        env->tr.limit < 103) {
+        goto fail;
+    }
+    io_offset = cpu_lduw_kernel_ra(env, env->tr.base + 0x66, retaddr);
+    io_offset += (addr >> 3);
+    /* Note: the check needs two bytes */
+    if ((io_offset + 1) > env->tr.limit) {
+        goto fail;
+    }
+    val = cpu_lduw_kernel_ra(env, env->tr.base + io_offset, retaddr);
+    val >>= (addr & 7);
+    mask = (1 << size) - 1;
+    /* all bits must be zero to allow the I/O */
+    if ((val & mask) != 0) {
+    fail:
+        raise_exception_err_ra(env, EXCP0D_GPF, 0, retaddr);
+    }
+}
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 860c75c2b1..bcc642bf6e 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -193,6 +193,7 @@  typedef struct DisasContext {
     { qemu_build_not_reached(); }
 
 #ifdef CONFIG_USER_ONLY
+STUB_HELPER(check_io, TCGv_env env, TCGv_i32 port, TCGv_i32 size)
 STUB_HELPER(clgi, TCGv_env env)
 STUB_HELPER(flush_page, TCGv_env env, TCGv addr)
 STUB_HELPER(hlt, TCGv_env env, TCGv_i32 pc_ofs)
@@ -217,6 +218,7 @@  static void gen_jr(DisasContext *s, TCGv dest);
 static void gen_jmp(DisasContext *s, target_ulong eip);
 static void gen_jmp_tb(DisasContext *s, target_ulong eip, int tb_num);
 static void gen_op(DisasContext *s1, int op, MemOp ot, int d);
+static void gen_exception_gpf(DisasContext *s);
 
 /* i386 arith/logic operations */
 enum {
@@ -681,6 +683,14 @@  static void gen_helper_out_func(MemOp ot, TCGv_i32 v, TCGv_i32 n)
 static bool gen_check_io(DisasContext *s, MemOp ot, TCGv_i32 port,
                          uint32_t svm_flags)
 {
+#ifdef CONFIG_USER_ONLY
+    /*
+     * We do not implement the iopriv(2) syscall, so the TSS check
+     * will always fail.
+     */
+    gen_exception_gpf(s);
+    return false;
+#else
     if (PE(s) && (CPL(s) > IOPL(s) || VM86(s))) {
         gen_helper_check_io(cpu_env, port, tcg_constant_i32(1 << ot));
     }
@@ -699,6 +709,7 @@  static bool gen_check_io(DisasContext *s, MemOp ot, TCGv_i32 port,
                                 tcg_constant_i32(next_eip - cur_eip));
     }
     return true;
+#endif
 }
 
 static inline void gen_movs(DisasContext *s, MemOp ot)