[Qemu-devel] [PATCH] target-s390x: fix LOAD MULTIPLE instruction on page boundary

[Aurelien Jarno]

When consecutive memory locations are on page boundary a page fault
might occur when using the LOAD MULTIPLE instruction. In that case real
hardware doesn't load any register.

This is an important detail in case the base register is in the list
of registers to be loaded. If a page fault occurs this register might be
overwritten and when the instruction is later restarted the wrong
base register value is useD.

Fix this by first loading all values from memory and then writing them
back to the registers.

This fixes random segmentation faults seen in the guest.

Cc: Alexander Graf <address@hidden>
Cc: Richard Henderson <address@hidden>
Signed-off-by: Aurelien Jarno <address@hidden>
---
 target-s390x/translate.c | 56 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 51 insertions(+), 5 deletions(-)

diff --git a/target-s390x/translate.c b/target-s390x/translate.c
index 52e106e..ddc78a9 100644
--- a/target-s390x/translate.c
+++ b/target-s390x/translate.c
@@ -2436,10 +2436,13 @@ static ExitStatus op_lm32(DisasContext *s, DisasOps *o)
     int r3 = get_field(s->fields, r3);
     TCGv_i64 t = tcg_temp_new_i64();
     TCGv_i64 t4 = tcg_const_i64(4);
+    TCGv_i64 tregs[16];
 
+    /* First load all the values from memory. If a page fault occurs the
+       registers should not be changed. */
     while (1) {
-        tcg_gen_qemu_ld32u(t, o->in2, get_mem_index(s));
-        store_reg32_i64(r1, t);
+        tregs[r1] = tcg_temp_new_i64();
+        tcg_gen_qemu_ld32u(tregs[r1], o->in2, get_mem_index(s));
         if (r1 == r3) {
             break;
         }
@@ -2447,6 +2450,18 @@ static ExitStatus op_lm32(DisasContext *s, DisasOps *o)
         r1 = (r1 + 1) & 15;
     }
 
+    /* When all the values have been loaded, write them back to the
+       registers. */
+    r1 = get_field(s->fields, r1);
+    while (1) {
+        store_reg32_i64(r1, tregs[r1]);
+        tcg_temp_free_i64(tregs[r1]);
+        if (r1 == r3) {
+            break;
+        }
+        r1 = (r1 + 1) & 15;
+    }
+
     tcg_temp_free_i64(t);
     tcg_temp_free_i64(t4);
     return NO_EXIT;
@@ -2458,10 +2473,13 @@ static ExitStatus op_lmh(DisasContext *s, DisasOps *o)
     int r3 = get_field(s->fields, r3);
     TCGv_i64 t = tcg_temp_new_i64();
     TCGv_i64 t4 = tcg_const_i64(4);
+    TCGv_i64 tregs[16];
 
+    /* First load all the values from memory. If a page fault occurs the
+       registers should not be changed. */
     while (1) {
-        tcg_gen_qemu_ld32u(t, o->in2, get_mem_index(s));
-        store_reg32h_i64(r1, t);
+        tregs[r1] = tcg_temp_new_i64();
+        tcg_gen_qemu_ld32u(tregs[r1], o->in2, get_mem_index(s));
         if (r1 == r3) {
             break;
         }
@@ -2469,6 +2487,18 @@ static ExitStatus op_lmh(DisasContext *s, DisasOps *o)
         r1 = (r1 + 1) & 15;
     }
 
+    /* When all the values have been loaded, write them back to the
+       registers. */
+    r1 = get_field(s->fields, r1);
+    while (1) {
+        store_reg32h_i64(r1, tregs[r1]);
+        tcg_temp_free_i64(tregs[r1]);
+        if (r1 == r3) {
+            break;
+        }
+        r1 = (r1 + 1) & 15;
+    }
+
     tcg_temp_free_i64(t);
     tcg_temp_free_i64(t4);
     return NO_EXIT;
@@ -2479,9 +2509,13 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
     int r1 = get_field(s->fields, r1);
     int r3 = get_field(s->fields, r3);
     TCGv_i64 t8 = tcg_const_i64(8);
+    TCGv_i64 tregs[16];
 
+    /* First load all the values from memory. If a page fault occurs the
+       registers should not be changed. */
     while (1) {
-        tcg_gen_qemu_ld64(regs[r1], o->in2, get_mem_index(s));
+        tregs[r1] = tcg_temp_new_i64();
+        tcg_gen_qemu_ld64(tregs[r1], o->in2, get_mem_index(s));
         if (r1 == r3) {
             break;
         }
@@ -2489,6 +2523,18 @@ static ExitStatus op_lm64(DisasContext *s, DisasOps *o)
         r1 = (r1 + 1) & 15;
     }
 
+    /* When all the values have been loaded, write them back to the
+       registers. */
+    r1 = get_field(s->fields, r1);
+    while (1) {
+        tcg_gen_mov_i64(regs[r1], tregs[r1]);
+        tcg_temp_free_i64(tregs[r1]);
+        if (r1 == r3) {
+            break;
+        }
+        r1 = (r1 + 1) & 15;
+    }
+
     tcg_temp_free_i64(t8);
     return NO_EXIT;
 }
-- 
2.1.4