Re: [Qemu-devel] Buggy wfi support for ARM user

["Laurent Desnogues" <laurent.desnogues@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 287 bytes --]

>> I propose that for user mode this instruction be considered
>> as a NOP.  Any thoughts?
>
> Allowing usermode to issue WFI sounds like a bug.

Here is a proposal that makes WFI no-ops for user mode emulation.
Comments welcome, especially as I am not used to send patches :)


Laurent

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: wfi.patch --]
[-- Type: text/x-patch; name=wfi.patch, Size: 2985 bytes --]

diff -ru -x .svn trunk/config-host.mak edit/config-host.mak
--- trunk/config-host.mak	2008-06-08 14:47:33.000000000 +0200
+++ edit/config-host.mak	2008-06-08 17:39:24.000000000 +0200
@@ -27,7 +27,7 @@
 CONFIG_VNC_TLS_CFLAGS= 
 CONFIG_VNC_TLS_LIBS=-lgnutls  
 VERSION=0.9.1
-SRC_PATH=/home/ldesnogu/work/Emu/qemu/trunk
+SRC_PATH=/home/ldesnogu/work/Emu/qemu/edit
 TARGET_DIRS=arm-linux-user
 CONFIG_SDL=yes
 SDL_LIBS=-L/usr/lib64 -lSDL -lpthread
diff -ru -x .svn trunk/target-arm/helpers.h edit/target-arm/helpers.h
--- trunk/target-arm/helpers.h	2008-04-17 23:10:21.000000000 +0200
+++ edit/target-arm/helpers.h	2008-06-08 17:51:23.000000000 +0200
@@ -122,7 +122,9 @@
 
 DEF_HELPER_1_3(sel_flags, uint32_t, (uint32_t, uint32_t, uint32_t))
 DEF_HELPER_0_1(exception, void, (uint32_t))
+#ifndef CONFIG_USER_ONLY
 DEF_HELPER_0_0(wfi, void, (void))
+#endif
 
 DEF_HELPER_0_2(cpsr_write, void, (uint32_t, uint32_t))
 DEF_HELPER_1_0(cpsr_read, uint32_t, (void))
diff -ru -x .svn trunk/target-arm/op_helper.c edit/target-arm/op_helper.c
--- trunk/target-arm/op_helper.c	2008-06-01 12:29:17.000000000 +0200
+++ edit/target-arm/op_helper.c	2008-06-08 17:50:55.000000000 +0200
@@ -247,12 +247,14 @@
     return res;
 }
 
+#ifndef CONFIG_USER_ONLY
 void HELPER(wfi)(void)
 {
     env->exception_index = EXCP_HLT;
     env->halted = 1;
     cpu_loop_exit();
 }
+#endif
 
 void HELPER(exception)(uint32_t excp)
 {
diff -ru -x .svn trunk/target-arm/translate.c edit/target-arm/translate.c
--- trunk/target-arm/translate.c	2008-06-07 13:59:11.000000000 +0200
+++ edit/target-arm/translate.c	2008-06-08 17:50:09.000000000 +0200
@@ -69,7 +69,10 @@
 
 /* These instructions trap after executing, so defer them until after the
    conditional executions state has been updated.  */
+/* for user mode hosted emulation wfi is treated as a no-op */
+#ifndef CONFIG_USER_ONLY
 #define DISAS_WFI 4
+#endif
 #define DISAS_SWI 5
 
 /* XXX: move that elsewhere */
@@ -2619,8 +2622,12 @@
     if ((insn & 0x0fff0fff) == 0x0e070f90
         || (insn & 0x0fff0fff) == 0x0e070f58) {
         /* Wait for interrupt.  */
+        /* for user mode hosted emulation wfi is treated as a no-op */
+        /* FIXME:  for ARMv7 this should always be a no-op */
+#ifndef CONFIG_USER_ONLY
         gen_set_pc_im(s->pc);
         s->is_jmp = DISAS_WFI;
+#endif
         return 0;
     }
     rd = (insn >> 12) & 0xf;
@@ -3499,8 +3506,11 @@
 {
     switch (val) {
     case 3: /* wfi */
+        /* for user mode hosted emulation wfi is treated as a no-op */
+#ifndef CONFIG_USER_ONLY
         gen_set_pc_im(s->pc);
         s->is_jmp = DISAS_WFI;
+#endif
         break;
     case 2: /* wfe */
     case 4: /* sev */
@@ -8712,9 +8722,11 @@
         case DISAS_TB_JUMP:
             /* nothing more to generate */
             break;
+#ifndef CONFIG_USER_ONLY
         case DISAS_WFI:
             gen_helper_wfi();
             break;
+#endif
         case DISAS_SWI:
             gen_exception(EXCP_SWI);
             break;