[Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Peter Maydell]

I was trying to reason about user-mode's handling of signal masks,
and I found our current code a bit confusing, so I cleaned it up.

At the moment for user-only mode cpu_resume_from_signal() takes a
usercontext pointer; if this is non-NULL then it has some awkward
OS and CPU specific code to set the signal mask from something
inside the usercontext before doing the same kind of siglongjmp()
that the softmmu cpu_resume_from_signal() does.

In fact the two use cases are completely separate:
 * almost all calls to cpu_resume_from_signal() pass a NULL puc
   argument (and most of those are softmmu-only anyway)
 * only the code path handle_cpu_signal -> page_unprotect ->
   tb_invalidate_phys_page -> cpu_resume_from_signal will pass
   a non-NULL puc.

The cleanups are:
 * pull the call to cpu_resume_from_signal() up through the
   callstack so we do the signal mask manipulation in
   handle_cpu_signal()
 * drop the OS/CPU spceific code to get a signal mask out of
   a usercontext, because in the specific case of handle_cpu_signal()
   we already have the signal mask value and can just use it
 * rename cpu_resume_from_signal() to cpu_loop_exit_noexc(),
   since all the remaining callsites are not in fact signal handlers
   or even called from signal handlers
 * get rid of an ugly TARGET_I386 ifdef in user-exec.c by moving
   the i386-specific code into its handle_mmu_fault hook.

Changes v1->v2:
 * patches 1-4 are the same and already reviewed
 * patch 5 is new, and just adds a clarifying comment to
   do_interrupt_user()
 * patch 6 is the old patch 5, and now sets env->exception_next_eip
   to -1 as a clear indication that the value is not going to be used
   (as noted in the comment in the new patch 5)

thanks
-- PMM


Peter Maydell (6):
  translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
  user-exec: Push resume-from-signal code out to handle_cpu_signal()
  cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
  user-exec: Don't reextract sigmask from usercontext pointer
  target-i386: Add comment about do_interrupt_user() next_eip argument
  target-i386: Move user-mode exception actions out of user-exec.c

 cpu-exec-common.c        |  8 ++---
 exec.c                   |  2 +-
 hw/i386/kvmvapic.c       |  2 +-
 include/exec/exec-all.h  |  2 +-
 target-i386/bpt_helper.c |  2 +-
 target-i386/helper.c     |  2 ++
 target-i386/seg_helper.c |  6 +++-
 target-lm32/helper.c     |  2 +-
 target-s390x/helper.c    |  2 +-
 target-xtensa/helper.c   |  2 +-
 translate-all.c          | 40 ++++++++++++---------
 translate-all.h          |  2 +-
 user-exec.c              | 93 +++++++++++++++++++++---------------------------
 13 files changed, 82 insertions(+), 83 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH v2 1/6] translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()

[Peter Maydell]

The user-mode-only function tb_invalidate_phys_page() is only
called from two places:
 * page_unprotect(), which passes in a non-zero pc, a puc pointer
   and the value 'true' for the locked argument
 * page_set_flags(), which passes in a zero pc, a NULL puc pointer
   and a 'false' locked argument

If the pc is non-zero then we may call cpu_resume_from_signal(),
which does a longjmp out of the calling code (and out of the
signal handler); this is to cover the case of a target CPU with
"precise self-modifying code" (currently only x86) executing
a store instruction which modifies code in the same TB as the
store itself. Rather than doing the longjump directly here,
return a flag to the caller which indicates whether the current
TB was modified, and move the longjump to page_unprotect.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 translate-all.c | 26 +++++++++++++++-----------
 1 file changed, 15 insertions(+), 11 deletions(-)

diff --git a/translate-all.c b/translate-all.c
index b54f472..4820d2e 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1438,10 +1438,13 @@ void tb_invalidate_phys_page_fast(tb_page_addr_t start, int len)
     }
 }
 #else
-/* Called with mmap_lock held.  */
-static void tb_invalidate_phys_page(tb_page_addr_t addr,
-                                    uintptr_t pc, void *puc,
-                                    bool locked)
+/* Called with mmap_lock held. If pc is not 0 then it indicates the
+ * host PC of the faulting store instruction that caused this invalidate.
+ * Returns true if the caller needs to abort execution of the current
+ * TB (because it was modified by this store and the guest CPU has
+ * precise-SMC semantics).
+ */
+static bool tb_invalidate_phys_page(tb_page_addr_t addr, uintptr_t pc)
 {
     TranslationBlock *tb;
     PageDesc *p;
@@ -1459,7 +1462,7 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
     addr &= TARGET_PAGE_MASK;
     p = page_find(addr >> TARGET_PAGE_BITS);
     if (!p) {
-        return;
+        return false;
     }
     tb = p->first_tb;
 #ifdef TARGET_HAS_PRECISE_SMC
@@ -1498,12 +1501,10 @@ static void tb_invalidate_phys_page(tb_page_addr_t addr,
            modifying the memory. It will ensure that it cannot modify
            itself */
         tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1);
-        if (locked) {
-            mmap_unlock();
-        }
-        cpu_resume_from_signal(cpu, puc);
+        return true;
     }
 #endif
+    return false;
 }
 #endif
 
@@ -1902,7 +1903,7 @@ void page_set_flags(target_ulong start, target_ulong end, int flags)
         if (!(p->flags & PAGE_WRITE) &&
             (flags & PAGE_WRITE) &&
             p->first_tb) {
-            tb_invalidate_phys_page(addr, 0, NULL, false);
+            tb_invalidate_phys_page(addr, 0);
         }
         p->flags = flags;
     }
@@ -1996,7 +1997,10 @@ int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
 
             /* and since the content will be modified, we must invalidate
                the corresponding translated code. */
-            tb_invalidate_phys_page(addr, pc, puc, true);
+            if (tb_invalidate_phys_page(addr, pc)) {
+                mmap_unlock();
+                cpu_resume_from_signal(current_cpu, puc);
+            }
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
 #endif
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/6] user-exec: Push resume-from-signal code out to handle_cpu_signal()

[Peter Maydell]

Since the only caller of page_unprotect() which might cause it to
need to call cpu_resume_from_signal() is handle_cpu_signal() in
the user-mode code, push the longjump handling out to that function.

Since this is the only caller of cpu_resume_from_signal() which
passes a non-NULL puc argument, split the non-NULL handling into
a new cpu_exit_tb_from_sighandler() function. This allows us
to merge the softmmu and usermode implementations of the
cpu_resume_from_signal() function, which are now identical.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 cpu-exec-common.c |  2 +-
 translate-all.c   | 12 ++++++++----
 translate-all.h   |  2 +-
 user-exec.c       | 41 +++++++++++++++++++++++++++++------------
 4 files changed, 39 insertions(+), 18 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index 6bdda6b..62f5d6b 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -28,7 +28,6 @@ CPUState *tcg_current_cpu;
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-#if defined(CONFIG_SOFTMMU)
 void cpu_resume_from_signal(CPUState *cpu, void *puc)
 {
     /* XXX: restore cpu registers saved in host registers */
@@ -37,6 +36,7 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc)
     siglongjmp(cpu->jmp_env, 1);
 }
 
+#if defined(CONFIG_SOFTMMU)
 void cpu_reloading_memory_map(void)
 {
     if (qemu_in_vcpu_thread()) {
diff --git a/translate-all.c b/translate-all.c
index 4820d2e..52a571e 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1955,7 +1955,7 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
             /* unprotect the page if it was put read-only because it
                contains translated code */
             if (!(p->flags & PAGE_WRITE)) {
-                if (!page_unprotect(addr, 0, NULL)) {
+                if (!page_unprotect(addr, 0)) {
                     return -1;
                 }
             }
@@ -1965,8 +1965,12 @@ int page_check_range(target_ulong start, target_ulong len, int flags)
 }
 
 /* called from signal handler: invalidate the code and unprotect the
-   page. Return TRUE if the fault was successfully handled. */
-int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
+ * page. Return 0 if the fault was not handled, 1 if it was handled,
+ * and 2 if it was handled but the caller must cause the TB to be
+ * immediately exited. (We can only return 2 if the 'pc' argument is
+ * non-zero.)
+ */
+int page_unprotect(target_ulong address, uintptr_t pc)
 {
     unsigned int prot;
     PageDesc *p;
@@ -1999,7 +2003,7 @@ int page_unprotect(target_ulong address, uintptr_t pc, void *puc)
                the corresponding translated code. */
             if (tb_invalidate_phys_page(addr, pc)) {
                 mmap_unlock();
-                cpu_resume_from_signal(current_cpu, puc);
+                return 2;
             }
 #ifdef DEBUG_TB_CHECK
             tb_invalidate_check(addr);
diff --git a/translate-all.h b/translate-all.h
index 0384640..ce6071b 100644
--- a/translate-all.h
+++ b/translate-all.h
@@ -27,7 +27,7 @@ void tb_invalidate_phys_range(tb_page_addr_t start, tb_page_addr_t end);
 void tb_check_watchpoint(CPUState *cpu);
 
 #ifdef CONFIG_USER_ONLY
-int page_unprotect(target_ulong address, uintptr_t pc, void *puc);
+int page_unprotect(target_ulong address, uintptr_t pc);
 #endif
 
 #endif /* TRANSLATE_ALL_H */
diff --git a/user-exec.c b/user-exec.c
index d8d597b..1d02e24 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -54,7 +54,7 @@ static void exception_action(CPUState *cpu)
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-void cpu_resume_from_signal(CPUState *cpu, void *puc)
+static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
 {
 #ifdef __linux__
     struct ucontext *uc = puc;
@@ -62,20 +62,18 @@ void cpu_resume_from_signal(CPUState *cpu, void *puc)
     struct sigcontext *uc = puc;
 #endif
 
-    if (puc) {
-        /* XXX: use siglongjmp ? */
+    /* XXX: use siglongjmp ? */
 #ifdef __linux__
 #ifdef __ia64
-        sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
+    sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
 #else
-        sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
+    sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
 #endif
 #elif defined(__OpenBSD__)
-        sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
+    sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
-    }
-    cpu->exception_index = -1;
-    siglongjmp(cpu->jmp_env, 1);
+
+    cpu_resume_from_signal(cpu, NULL);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
@@ -95,9 +93,28 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
            pc, address, is_write, *(unsigned long *)old_set);
 #endif
     /* XXX: locking issue */
-    if (is_write && h2g_valid(address)
-        && page_unprotect(h2g(address), pc, puc)) {
-        return 1;
+    if (is_write && h2g_valid(address)) {
+        switch (page_unprotect(h2g(address), pc)) {
+        case 0:
+            /* Fault not caused by a page marked unwritable to protect
+             * cached translations, must be the guest binary's problem
+             */
+            break;
+        case 1:
+            /* Fault caused by protection of cached translation; TBs
+             * invalidated, so resume execution
+             */
+            return 1;
+        case 2:
+            /* Fault caused by protection of cached translation, and the
+             * currently executing TB was modified and must be exited
+             * immediately.
+             */
+            cpu_exit_tb_from_sighandler(current_cpu, puc);
+            g_assert_not_reached();
+        default:
+            g_assert_not_reached();
+        }
     }
 
     /* Convert forcefully to guest address space, invalid addresses
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/6] cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()

[Peter Maydell]

The function cpu_resume_from_signal() is now always called with a
NULL puc argument, and is rather misnamed since it is never called
from a signal handler. It is essentially forcing an exit to the
top level cpu loop but without raising any exception, so rename
it to cpu_loop_exit_noexc() and drop the useless unused argument.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 cpu-exec-common.c        | 6 ++----
 exec.c                   | 2 +-
 hw/i386/kvmvapic.c       | 2 +-
 include/exec/exec-all.h  | 2 +-
 target-i386/bpt_helper.c | 2 +-
 target-lm32/helper.c     | 2 +-
 target-s390x/helper.c    | 2 +-
 target-xtensa/helper.c   | 2 +-
 translate-all.c          | 4 ++--
 user-exec.c              | 2 +-
 10 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/cpu-exec-common.c b/cpu-exec-common.c
index 62f5d6b..fac3aa7 100644
--- a/cpu-exec-common.c
+++ b/cpu-exec-common.c
@@ -25,10 +25,8 @@
 bool exit_request;
 CPUState *tcg_current_cpu;
 
-/* exit the current TB from a signal handler. The host registers are
-   restored in a state compatible with the CPU emulator
- */
-void cpu_resume_from_signal(CPUState *cpu, void *puc)
+/* exit the current TB, but without causing any exception to be raised */
+void cpu_loop_exit_noexc(CPUState *cpu)
 {
     /* XXX: restore cpu registers saved in host registers */
 
diff --git a/exec.c b/exec.c
index ee45472..c5c97ac 100644
--- a/exec.c
+++ b/exec.c
@@ -2121,7 +2121,7 @@ static void check_watchpoint(int offset, int len, MemTxAttrs attrs, int flags)
                 } else {
                     cpu_get_tb_cpu_state(env, &pc, &cs_base, &cpu_flags);
                     tb_gen_code(cpu, pc, cs_base, cpu_flags, 1);
-                    cpu_resume_from_signal(cpu, NULL);
+                    cpu_loop_exit_noexc(cpu);
                 }
             }
         } else {
diff --git a/hw/i386/kvmvapic.c b/hw/i386/kvmvapic.c
index f14445d..ee2f3fa 100644
--- a/hw/i386/kvmvapic.c
+++ b/hw/i386/kvmvapic.c
@@ -447,7 +447,7 @@ static void patch_instruction(VAPICROMState *s, X86CPU *cpu, target_ulong ip)
 
     if (!kvm_enabled()) {
         tb_gen_code(cs, current_pc, current_cs_base, current_flags, 1);
-        cpu_resume_from_signal(cs, NULL);
+        cpu_loop_exit_noexc(cs);
     }
 }
 
diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 85528f9..1359f14 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -73,7 +73,7 @@ void restore_state_to_opc(CPUArchState *env, struct TranslationBlock *tb,
 void cpu_gen_init(void);
 bool cpu_restore_state(CPUState *cpu, uintptr_t searched_pc);
 
-void QEMU_NORETURN cpu_resume_from_signal(CPUState *cpu, void *puc);
+void QEMU_NORETURN cpu_loop_exit_noexc(CPUState *cpu);
 void QEMU_NORETURN cpu_io_recompile(CPUState *cpu, uintptr_t retaddr);
 TranslationBlock *tb_gen_code(CPUState *cpu,
                               target_ulong pc, target_ulong cs_base,
diff --git a/target-i386/bpt_helper.c b/target-i386/bpt_helper.c
index f47df19..458170e 100644
--- a/target-i386/bpt_helper.c
+++ b/target-i386/bpt_helper.c
@@ -217,7 +217,7 @@ void breakpoint_handler(CPUState *cs)
             if (check_hw_breakpoints(env, false)) {
                 raise_exception(env, EXCP01_DB);
             } else {
-                cpu_resume_from_signal(cs, NULL);
+                cpu_loop_exit_noexc(cs);
             }
         }
     } else {
diff --git a/target-lm32/helper.c b/target-lm32/helper.c
index 655248f..accfa7c 100644
--- a/target-lm32/helper.c
+++ b/target-lm32/helper.c
@@ -140,7 +140,7 @@ void lm32_debug_excp_handler(CPUState *cs)
             if (check_watchpoints(env)) {
                 raise_exception(env, EXCP_WATCHPOINT);
             } else {
-                cpu_resume_from_signal(cs, NULL);
+                cpu_loop_exit_noexc(cs);
             }
         }
     } else {
diff --git a/target-s390x/helper.c b/target-s390x/helper.c
index 92abe7e..f1e0a43 100644
--- a/target-s390x/helper.c
+++ b/target-s390x/helper.c
@@ -686,7 +686,7 @@ void s390x_cpu_debug_excp_handler(CPUState *cs)
            will be triggered, it will call load_psw which will recompute
            the watchpoints.  */
         cpu_watchpoint_remove_all(cs, BP_CPU);
-        cpu_resume_from_signal(cs, NULL);
+        cpu_loop_exit_noexc(cs);
     }
 }
 #endif /* CONFIG_USER_ONLY */
diff --git a/target-xtensa/helper.c b/target-xtensa/helper.c
index 839f4a7..768b32c 100644
--- a/target-xtensa/helper.c
+++ b/target-xtensa/helper.c
@@ -108,7 +108,7 @@ void xtensa_breakpoint_handler(CPUState *cs)
             if (cause) {
                 debug_exception_env(env, cause);
             }
-            cpu_resume_from_signal(cs, NULL);
+            cpu_loop_exit_noexc(cs);
         }
     }
 }
diff --git a/translate-all.c b/translate-all.c
index 52a571e..3a0d10f 100644
--- a/translate-all.c
+++ b/translate-all.c
@@ -1394,7 +1394,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end,
            modifying the memory. It will ensure that it cannot modify
            itself */
         tb_gen_code(cpu, current_pc, current_cs_base, current_flags, 1);
-        cpu_resume_from_signal(cpu, NULL);
+        cpu_loop_exit_noexc(cpu);
     }
 #endif
 }
@@ -1654,7 +1654,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
        repeating the fault, which is horribly inefficient.
        Better would be to execute just this insn uncached, or generate a
        second new TB.  */
-    cpu_resume_from_signal(cpu, NULL);
+    cpu_loop_exit_noexc(cpu);
 }
 
 void tb_flush_jmp_cache(CPUState *cpu, target_ulong addr)
diff --git a/user-exec.c b/user-exec.c
index 1d02e24..40b5e7c 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -73,7 +73,7 @@ static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
     sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
 #endif
 
-    cpu_resume_from_signal(cpu, NULL);
+    cpu_loop_exit_noexc(cpu);
 }
 
 /* 'pc' is the host PC at which the exception was raised. 'address' is
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/6] user-exec: Don't reextract sigmask from usercontext pointer

[Peter Maydell]

Extracting the old signal mask from the usercontext pointer passed to
a signal handler is a pain because it is OS and CPU dependent.
Since we've already done it once and passed it to handle_cpu_signal(),
there's no need to do it again in cpu_exit_tb_from_sighandler().
This then means we don't need to pass a usercontext pointer in to
handle_cpu_signal() at all.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>
---
 user-exec.c | 48 ++++++++++++++++--------------------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/user-exec.c b/user-exec.c
index 40b5e7c..ad669f4 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -54,25 +54,10 @@ static void exception_action(CPUState *cpu)
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
-static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
+static void cpu_exit_tb_from_sighandler(CPUState *cpu, sigset_t *old_set)
 {
-#ifdef __linux__
-    struct ucontext *uc = puc;
-#elif defined(__OpenBSD__)
-    struct sigcontext *uc = puc;
-#endif
-
     /* XXX: use siglongjmp ? */
-#ifdef __linux__
-#ifdef __ia64
-    sigprocmask(SIG_SETMASK, (sigset_t *)&uc->uc_sigmask, NULL);
-#else
-    sigprocmask(SIG_SETMASK, &uc->uc_sigmask, NULL);
-#endif
-#elif defined(__OpenBSD__)
-    sigprocmask(SIG_SETMASK, &uc->sc_mask, NULL);
-#endif
-
+    sigprocmask(SIG_SETMASK, old_set, NULL);
     cpu_loop_exit_noexc(cpu);
 }
 
@@ -81,8 +66,7 @@ static void cpu_exit_tb_from_sighandler(CPUState *cpu, void *puc)
    write caused the exception and otherwise 0'. 'old_set' is the
    signal set which should be restored */
 static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
-                                    int is_write, sigset_t *old_set,
-                                    void *puc)
+                                    int is_write, sigset_t *old_set)
 {
     CPUState *cpu;
     CPUClass *cc;
@@ -110,7 +94,7 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
              * currently executing TB was modified and must be exited
              * immediately.
              */
-            cpu_exit_tb_from_sighandler(current_cpu, puc);
+            cpu_exit_tb_from_sighandler(current_cpu, old_set);
             g_assert_not_reached();
         default:
             g_assert_not_reached();
@@ -204,7 +188,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              trapno == 0xe ?
                              (ERROR_sig(uc) >> 1) & 1 : 0,
-                             &MASK_sig(uc), puc);
+                             &MASK_sig(uc));
 }
 
 #elif defined(__x86_64__)
@@ -250,7 +234,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              TRAP_sig(uc) == 0xe ?
                              (ERROR_sig(uc) >> 1) & 1 : 0,
-                             &MASK_sig(uc), puc);
+                             &MASK_sig(uc));
 }
 
 #elif defined(_ARCH_PPC)
@@ -366,7 +350,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 #endif
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__alpha__)
@@ -397,7 +381,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 #elif defined(__sparc__)
 
@@ -457,7 +441,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
         }
     }
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, sigmask, NULL);
+                             is_write, sigmask);
 }
 
 #elif defined(__arm__)
@@ -492,7 +476,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     is_write = extract32(uc->uc_mcontext.error_code, 11, 1);
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
-                             &uc->uc_sigmask, puc);
+                             &uc->uc_sigmask);
 }
 
 #elif defined(__aarch64__)
@@ -520,7 +504,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
                 || (insn & 0x3a400000) == 0x28000000); /* C3.3.7,14-16 */
 
     return handle_cpu_signal(pc, (uintptr_t)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__mc68000)
@@ -538,7 +522,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     is_write = 0;
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
                              is_write,
-                             &uc->uc_sigmask, puc);
+                             &uc->uc_sigmask);
 }
 
 #elif defined(__ia64)
@@ -573,7 +557,7 @@ int cpu_signal_handler(int host_signum, void *pinfo, void *puc)
     }
     return handle_cpu_signal(ip, (unsigned long)info->si_addr,
                              is_write,
-                             (sigset_t *)&uc->uc_sigmask, puc);
+                             (sigset_t *)&uc->uc_sigmask);
 }
 
 #elif defined(__s390__)
@@ -626,7 +610,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
         break;
     }
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__mips__)
@@ -642,7 +626,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     /* XXX: compute is_write */
     is_write = 0;
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #elif defined(__hppa__)
@@ -684,7 +668,7 @@ int cpu_signal_handler(int host_signum, void *pinfo,
     }
 
     return handle_cpu_signal(pc, (unsigned long)info->si_addr,
-                             is_write, &uc->uc_sigmask, puc);
+                             is_write, &uc->uc_sigmask);
 }
 
 #else
-- 
1.9.1

[Qemu-devel] [PATCH v2 5/6] target-i386: Add comment about do_interrupt_user() next_eip argument

[Peter Maydell]

Add a comment to do_interrupt_user() along the same lines as the
existing one for do_interrupt_all() noting that the next_eip
argument is not used unless is_int is true or intno is EXCP_SYSCALL.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-i386/seg_helper.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
index b5f3d72..860f948 100644
--- a/target-i386/seg_helper.c
+++ b/target-i386/seg_helper.c
@@ -1128,7 +1128,11 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
 }
 
 #if defined(CONFIG_USER_ONLY)
-/* fake user mode interrupt */
+/* fake user mode interrupt. is_int is TRUE if coming from the int
+ * instruction. next_eip is the env->eip value AFTER the interrupt
+ * instruction. It is only relevant if is_int is TRUE or if intno
+ * is EXCP_SYSCALL.
+ */
 static void do_interrupt_user(CPUX86State *env, int intno, int is_int,
                               int error_code, target_ulong next_eip)
 {
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 5/6] target-i386: Add comment about do_interrupt_user() next_eip argument

[Sergey Fedorov]

On 17/05/16 17:18, Peter Maydell wrote:
> Add a comment to do_interrupt_user() along the same lines as the
> existing one for do_interrupt_all() noting that the next_eip
> argument is not used unless is_int is true or intno is EXCP_SYSCALL.
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>

> ---
>  target-i386/seg_helper.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target-i386/seg_helper.c b/target-i386/seg_helper.c
> index b5f3d72..860f948 100644
> --- a/target-i386/seg_helper.c
> +++ b/target-i386/seg_helper.c
> @@ -1128,7 +1128,11 @@ static void do_interrupt_real(CPUX86State *env, int intno, int is_int,
>  }
>  
>  #if defined(CONFIG_USER_ONLY)
> -/* fake user mode interrupt */
> +/* fake user mode interrupt. is_int is TRUE if coming from the int
> + * instruction. next_eip is the env->eip value AFTER the interrupt
> + * instruction. It is only relevant if is_int is TRUE or if intno
> + * is EXCP_SYSCALL.
> + */
>  static void do_interrupt_user(CPUX86State *env, int intno, int is_int,
>                                int error_code, target_ulong next_eip)
>  {

[Qemu-devel] [PATCH v2 6/6] target-i386: Move user-mode exception actions out of user-exec.c

[Peter Maydell]

The exception_action() function in user-exec.c is just a call to
cpu_loop_exit() for every target CPU except i386.  Since this
function is only called if the target's handle_mmu_fault() hook has
indicated an MMU fault, and that hook is only called from the
handle_cpu_signal() code path, we can simply move the x86-specific
setup into that hook, which allows us to remove the TARGET_I386
ifdef from user-exec.c.

Of the actions that were done by the call to raise_interrupt_err():
 * cpu_svm_check_intercept_param() is a no-op in user mode
 * check_exception() is a no-op since double faults are impossible
   for user-mode
 * assignments to cs->exception_index and env->error_code are no-ops
 * assigning to env->exception_next_eip is unnecessary because it
   is not used unless env->exception_is_int is true
 * cpu_loop_exit_restore() is equivalent to cpu_loop_exit() since
   pc is 0
which leaves just setting env_>exception_is_int as the action that
needs to be added to x86_cpu_handle_mmu_fault().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target-i386/helper.c |  2 ++
 user-exec.c          | 16 +---------------
 2 files changed, 3 insertions(+), 15 deletions(-)

diff --git a/target-i386/helper.c b/target-i386/helper.c
index bf3e762..81fad6d 100644
--- a/target-i386/helper.c
+++ b/target-i386/helper.c
@@ -700,6 +700,8 @@ int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr,
     env->error_code = (is_write << PG_ERROR_W_BIT);
     env->error_code |= PG_ERROR_U_MASK;
     cs->exception_index = EXCP0E_PAGE;
+    env->exception_is_int = 0;
+    env->exception_next_eip = -1;
     return 1;
 }
 
diff --git a/user-exec.c b/user-exec.c
index ad669f4..439bb37 100644
--- a/user-exec.c
+++ b/user-exec.c
@@ -39,18 +39,6 @@
 
 //#define DEBUG_SIGNAL
 
-static void exception_action(CPUState *cpu)
-{
-#if defined(TARGET_I386)
-    X86CPU *x86_cpu = X86_CPU(cpu);
-    CPUX86State *env1 = &x86_cpu->env;
-
-    raise_exception_err(env1, cpu->exception_index, env1->error_code);
-#else
-    cpu_loop_exit(cpu);
-#endif
-}
-
 /* exit the current TB from a signal handler. The host registers are
    restored in a state compatible with the CPU emulator
  */
@@ -119,10 +107,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
     /* now we have a real cpu fault */
     cpu_restore_state(cpu, pc);
 
-    /* we restore the process signal mask as the sigreturn should
-       do it (XXX: use sigsetjmp) */
     sigprocmask(SIG_SETMASK, old_set, NULL);
-    exception_action(cpu);
+    cpu_loop_exit(cpu);
 
     /* never comes here */
     return 1;
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 6/6] target-i386: Move user-mode exception actions out of user-exec.c

[Sergey Fedorov]

On 17/05/16 17:18, Peter Maydell wrote:
> The exception_action() function in user-exec.c is just a call to
> cpu_loop_exit() for every target CPU except i386.  Since this
> function is only called if the target's handle_mmu_fault() hook has
> indicated an MMU fault, and that hook is only called from the
> handle_cpu_signal() code path, we can simply move the x86-specific
> setup into that hook, which allows us to remove the TARGET_I386
> ifdef from user-exec.c.
>
> Of the actions that were done by the call to raise_interrupt_err():
>  * cpu_svm_check_intercept_param() is a no-op in user mode
>  * check_exception() is a no-op since double faults are impossible
>    for user-mode
>  * assignments to cs->exception_index and env->error_code are no-ops
>  * assigning to env->exception_next_eip is unnecessary because it
>    is not used unless env->exception_is_int is true
>  * cpu_loop_exit_restore() is equivalent to cpu_loop_exit() since
>    pc is 0
> which leaves just setting env_>exception_is_int as the action that
> needs to be added to x86_cpu_handle_mmu_fault().
>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Sergey Fedorov <sergey.fedorov@linaro.org>

> ---
>  target-i386/helper.c |  2 ++
>  user-exec.c          | 16 +---------------
>  2 files changed, 3 insertions(+), 15 deletions(-)
>
> diff --git a/target-i386/helper.c b/target-i386/helper.c
> index bf3e762..81fad6d 100644
> --- a/target-i386/helper.c
> +++ b/target-i386/helper.c
> @@ -700,6 +700,8 @@ int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr,
>      env->error_code = (is_write << PG_ERROR_W_BIT);
>      env->error_code |= PG_ERROR_U_MASK;
>      cs->exception_index = EXCP0E_PAGE;
> +    env->exception_is_int = 0;
> +    env->exception_next_eip = -1;
>      return 1;
>  }
>  
> diff --git a/user-exec.c b/user-exec.c
> index ad669f4..439bb37 100644
> --- a/user-exec.c
> +++ b/user-exec.c
> @@ -39,18 +39,6 @@
>  
>  //#define DEBUG_SIGNAL
>  
> -static void exception_action(CPUState *cpu)
> -{
> -#if defined(TARGET_I386)
> -    X86CPU *x86_cpu = X86_CPU(cpu);
> -    CPUX86State *env1 = &x86_cpu->env;
> -
> -    raise_exception_err(env1, cpu->exception_index, env1->error_code);
> -#else
> -    cpu_loop_exit(cpu);
> -#endif
> -}
> -
>  /* exit the current TB from a signal handler. The host registers are
>     restored in a state compatible with the CPU emulator
>   */
> @@ -119,10 +107,8 @@ static inline int handle_cpu_signal(uintptr_t pc, unsigned long address,
>      /* now we have a real cpu fault */
>      cpu_restore_state(cpu, pc);
>  
> -    /* we restore the process signal mask as the sigreturn should
> -       do it (XXX: use sigsetjmp) */
>      sigprocmask(SIG_SETMASK, old_set, NULL);
> -    exception_action(cpu);
> +    cpu_loop_exit(cpu);
>  
>      /* never comes here */
>      return 1;

Re: [Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Peter Maydell]

Ping!

thanks
-- PMM

On 17 May 2016 at 15:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> I was trying to reason about user-mode's handling of signal masks,
> and I found our current code a bit confusing, so I cleaned it up.
>
> At the moment for user-only mode cpu_resume_from_signal() takes a
> usercontext pointer; if this is non-NULL then it has some awkward
> OS and CPU specific code to set the signal mask from something
> inside the usercontext before doing the same kind of siglongjmp()
> that the softmmu cpu_resume_from_signal() does.
>
> In fact the two use cases are completely separate:
>  * almost all calls to cpu_resume_from_signal() pass a NULL puc
>    argument (and most of those are softmmu-only anyway)
>  * only the code path handle_cpu_signal -> page_unprotect ->
>    tb_invalidate_phys_page -> cpu_resume_from_signal will pass
>    a non-NULL puc.
>
> The cleanups are:
>  * pull the call to cpu_resume_from_signal() up through the
>    callstack so we do the signal mask manipulation in
>    handle_cpu_signal()
>  * drop the OS/CPU spceific code to get a signal mask out of
>    a usercontext, because in the specific case of handle_cpu_signal()
>    we already have the signal mask value and can just use it
>  * rename cpu_resume_from_signal() to cpu_loop_exit_noexc(),
>    since all the remaining callsites are not in fact signal handlers
>    or even called from signal handlers
>  * get rid of an ugly TARGET_I386 ifdef in user-exec.c by moving
>    the i386-specific code into its handle_mmu_fault hook.
>
> Changes v1->v2:
>  * patches 1-4 are the same and already reviewed
>  * patch 5 is new, and just adds a clarifying comment to
>    do_interrupt_user()
>  * patch 6 is the old patch 5, and now sets env->exception_next_eip
>    to -1 as a clear indication that the value is not going to be used
>    (as noted in the comment in the new patch 5)
>
> thanks
> -- PMM
>
>
> Peter Maydell (6):
>   translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
>   user-exec: Push resume-from-signal code out to handle_cpu_signal()
>   cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
>   user-exec: Don't reextract sigmask from usercontext pointer
>   target-i386: Add comment about do_interrupt_user() next_eip argument
>   target-i386: Move user-mode exception actions out of user-exec.c
>
>  cpu-exec-common.c        |  8 ++---
>  exec.c                   |  2 +-
>  hw/i386/kvmvapic.c       |  2 +-
>  include/exec/exec-all.h  |  2 +-
>  target-i386/bpt_helper.c |  2 +-
>  target-i386/helper.c     |  2 ++
>  target-i386/seg_helper.c |  6 +++-
>  target-lm32/helper.c     |  2 +-
>  target-s390x/helper.c    |  2 +-
>  target-xtensa/helper.c   |  2 +-
>  translate-all.c          | 40 ++++++++++++---------
>  translate-all.h          |  2 +-
>  user-exec.c              | 93 +++++++++++++++++++++---------------------------
>  13 files changed, 82 insertions(+), 83 deletions(-)
>
> --
> 1.9.1

Re: [Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Peter Maydell]

On 6 June 2016 at 15:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> Ping!

Thanks for the review, Sergey. Unless anybody else wants to review
or wants to take it through their tree (Riku?), I propose to apply
this to master sometime later this week.

thanks
-- PMM

> On 17 May 2016 at 15:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>> I was trying to reason about user-mode's handling of signal masks,
>> and I found our current code a bit confusing, so I cleaned it up.
>>
>> At the moment for user-only mode cpu_resume_from_signal() takes a
>> usercontext pointer; if this is non-NULL then it has some awkward
>> OS and CPU specific code to set the signal mask from something
>> inside the usercontext before doing the same kind of siglongjmp()
>> that the softmmu cpu_resume_from_signal() does.
>>
>> In fact the two use cases are completely separate:
>>  * almost all calls to cpu_resume_from_signal() pass a NULL puc
>>    argument (and most of those are softmmu-only anyway)
>>  * only the code path handle_cpu_signal -> page_unprotect ->
>>    tb_invalidate_phys_page -> cpu_resume_from_signal will pass
>>    a non-NULL puc.
>>
>> The cleanups are:
>>  * pull the call to cpu_resume_from_signal() up through the
>>    callstack so we do the signal mask manipulation in
>>    handle_cpu_signal()
>>  * drop the OS/CPU spceific code to get a signal mask out of
>>    a usercontext, because in the specific case of handle_cpu_signal()
>>    we already have the signal mask value and can just use it
>>  * rename cpu_resume_from_signal() to cpu_loop_exit_noexc(),
>>    since all the remaining callsites are not in fact signal handlers
>>    or even called from signal handlers
>>  * get rid of an ugly TARGET_I386 ifdef in user-exec.c by moving
>>    the i386-specific code into its handle_mmu_fault hook.
>>
>> Changes v1->v2:
>>  * patches 1-4 are the same and already reviewed
>>  * patch 5 is new, and just adds a clarifying comment to
>>    do_interrupt_user()
>>  * patch 6 is the old patch 5, and now sets env->exception_next_eip
>>    to -1 as a clear indication that the value is not going to be used
>>    (as noted in the comment in the new patch 5)
>>
>> thanks
>> -- PMM
>>
>>
>> Peter Maydell (6):
>>   translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
>>   user-exec: Push resume-from-signal code out to handle_cpu_signal()
>>   cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
>>   user-exec: Don't reextract sigmask from usercontext pointer
>>   target-i386: Add comment about do_interrupt_user() next_eip argument
>>   target-i386: Move user-mode exception actions out of user-exec.c
>>
>>  cpu-exec-common.c        |  8 ++---
>>  exec.c                   |  2 +-
>>  hw/i386/kvmvapic.c       |  2 +-
>>  include/exec/exec-all.h  |  2 +-
>>  target-i386/bpt_helper.c |  2 +-
>>  target-i386/helper.c     |  2 ++
>>  target-i386/seg_helper.c |  6 +++-
>>  target-lm32/helper.c     |  2 +-
>>  target-s390x/helper.c    |  2 +-
>>  target-xtensa/helper.c   |  2 +-
>>  translate-all.c          | 40 ++++++++++++---------
>>  translate-all.h          |  2 +-
>>  user-exec.c              | 93 +++++++++++++++++++++---------------------------
>>  13 files changed, 82 insertions(+), 83 deletions(-)
>>
>> --
>> 1.9.1

Re: [Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Eduardo Habkost]

On Mon, Jun 06, 2016 at 05:57:35PM +0100, Peter Maydell wrote:
> On 6 June 2016 at 15:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Ping!
> 
> Thanks for the review, Sergey. Unless anybody else wants to review
> or wants to take it through their tree (Riku?), I propose to apply
> this to master sometime later this week.

I couldn't review it completely, but I'm OK with this going
through your tree.

Acked-by: Eduardo Habkost <ehabkost@redhat.com>

-- 
Eduardo

Re: [Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Riku Voipio]

On Mon, Jun 06, 2016 at 05:57:35PM +0100, Peter Maydell wrote:
> On 6 June 2016 at 15:55, Peter Maydell <peter.maydell@linaro.org> wrote:
> > Ping!
> 
> Thanks for the review, Sergey. Unless anybody else wants to review
> or wants to take it through their tree (Riku?), I propose to apply
> this to master sometime later this week.

Feel free to apply these yourself,
Acked-by: Riku Voipio <riku.voipio@linaro.org>

> thanks
> -- PMM
> 
> > On 17 May 2016 at 15:18, Peter Maydell <peter.maydell@linaro.org> wrote:
> >> I was trying to reason about user-mode's handling of signal masks,
> >> and I found our current code a bit confusing, so I cleaned it up.
> >>
> >> At the moment for user-only mode cpu_resume_from_signal() takes a
> >> usercontext pointer; if this is non-NULL then it has some awkward
> >> OS and CPU specific code to set the signal mask from something
> >> inside the usercontext before doing the same kind of siglongjmp()
> >> that the softmmu cpu_resume_from_signal() does.
> >>
> >> In fact the two use cases are completely separate:
> >>  * almost all calls to cpu_resume_from_signal() pass a NULL puc
> >>    argument (and most of those are softmmu-only anyway)
> >>  * only the code path handle_cpu_signal -> page_unprotect ->
> >>    tb_invalidate_phys_page -> cpu_resume_from_signal will pass
> >>    a non-NULL puc.
> >>
> >> The cleanups are:
> >>  * pull the call to cpu_resume_from_signal() up through the
> >>    callstack so we do the signal mask manipulation in
> >>    handle_cpu_signal()
> >>  * drop the OS/CPU spceific code to get a signal mask out of
> >>    a usercontext, because in the specific case of handle_cpu_signal()
> >>    we already have the signal mask value and can just use it
> >>  * rename cpu_resume_from_signal() to cpu_loop_exit_noexc(),
> >>    since all the remaining callsites are not in fact signal handlers
> >>    or even called from signal handlers
> >>  * get rid of an ugly TARGET_I386 ifdef in user-exec.c by moving
> >>    the i386-specific code into its handle_mmu_fault hook.
> >>
> >> Changes v1->v2:
> >>  * patches 1-4 are the same and already reviewed
> >>  * patch 5 is new, and just adds a clarifying comment to
> >>    do_interrupt_user()
> >>  * patch 6 is the old patch 5, and now sets env->exception_next_eip
> >>    to -1 as a clear indication that the value is not going to be used
> >>    (as noted in the comment in the new patch 5)
> >>
> >> thanks
> >> -- PMM
> >>
> >>
> >> Peter Maydell (6):
> >>   translate-all.c: Don't pass puc, locked to tb_invalidate_phys_page()
> >>   user-exec: Push resume-from-signal code out to handle_cpu_signal()
> >>   cpu-exec: Rename cpu_resume_from_signal() to cpu_loop_exit_noexc()
> >>   user-exec: Don't reextract sigmask from usercontext pointer
> >>   target-i386: Add comment about do_interrupt_user() next_eip argument
> >>   target-i386: Move user-mode exception actions out of user-exec.c
> >>
> >>  cpu-exec-common.c        |  8 ++---
> >>  exec.c                   |  2 +-
> >>  hw/i386/kvmvapic.c       |  2 +-
> >>  include/exec/exec-all.h  |  2 +-
> >>  target-i386/bpt_helper.c |  2 +-
> >>  target-i386/helper.c     |  2 ++
> >>  target-i386/seg_helper.c |  6 +++-
> >>  target-lm32/helper.c     |  2 +-
> >>  target-s390x/helper.c    |  2 +-
> >>  target-xtensa/helper.c   |  2 +-
> >>  translate-all.c          | 40 ++++++++++++---------
> >>  translate-all.h          |  2 +-
> >>  user-exec.c              | 93 +++++++++++++++++++++---------------------------
> >>  13 files changed, 82 insertions(+), 83 deletions(-)
> >>
> >> --
> >> 1.9.1

Re: [Qemu-devel] [PATCH v2 0/6] user-exec: cpu_resume_from_signal() cleanups

[Peter Maydell]

On 7 June 2016 at 08:59, Riku Voipio <riku.voipio@iki.fi> wrote:
> On Mon, Jun 06, 2016 at 05:57:35PM +0100, Peter Maydell wrote:
>> On 6 June 2016 at 15:55, Peter Maydell <peter.maydell@linaro.org> wrote:
>> > Ping!
>>
>> Thanks for the review, Sergey. Unless anybody else wants to review
>> or wants to take it through their tree (Riku?), I propose to apply
>> this to master sometime later this week.
>
> Feel free to apply these yourself,
> Acked-by: Riku Voipio <riku.voipio@linaro.org>

Applied to master, thanks.

-- PMM