QEMU-Devel Archive on lore.kernel.org
 help / color / Atom feed
* [PULL 00/31] s390x/tcg update
@ 2019-10-10 11:33 David Hildenbrand
  2019-10-10 11:33 ` [PULL 01/31] s390x/mmu: Drop debug logging from MMU code David Hildenbrand
                   ` (32 more replies)
  0 siblings, 33 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Hi Peter,

The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-10-08 16:08:35 +0100)

are available in the Git repository at:

  https://github.com/davidhildenbrand/qemu.git tags/s390x-tcg-2019-10-10

for you to fetch changes up to 1f6493be088da969efeeee0100ea4330fb804cdf:

  s390x/tcg: MVCL: Exit to main loop if requested (2019-10-10 12:27:15 +0200)

----------------------------------------------------------------
- MMU DAT translation rewrite and cleanup
- Implement more TCG CPU features related to the MMU (e.g., IEP)
- Add the current instruction length to unwind data and clean up
- Resolve one TODO for the MVCL instruction

----------------------------------------------------------------
David Hildenbrand (13):
  s390x/mmu: Drop debug logging from MMU code
  s390x/mmu: Move DAT protection handling out of mmu_translate_asce()
  s390x/mmu: Inject DAT exceptions from a single place
  s390x/mmu: Inject PGM_ADDRESSING on bogus table addresses
  s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte()
  s390x/mmu: DAT table definition overhaul
  s390x/mmu: Convert to non-recursive page table walk
  s390x/mmu: Add EDAT2 translation support
  s390x/mmu: Implement ESOP-2 and
    access-exception-fetch/store-indication facility
  s390x/mmu: Implement Instruction-Execution-Protection Facility
  s390x/cpumodel: Prepare for changes of QEMU model
  s390x/cpumodel: Add new TCG features to QEMU cpu model
  s390x/tcg: MVCL: Exit to main loop if requested

Richard Henderson (18):
  target/s390x: Add ilen to unwind data
  target/s390x: Remove ilen parameter from tcg_s390_program_interrupt
  target/s390x: Remove ilen parameter from s390_program_interrupt
  target/s390x: Use tcg_s390_program_interrupt in TCG helpers
  target/s390x: Push trigger_pgm_exception lower in s390_cpu_tlb_fill
  target/s390x: Handle tec in s390_cpu_tlb_fill
  target/s390x: Return exception from mmu_translate_real
  target/s390x: Remove exc argument to mmu_translate_asce
  target/s390x: Return exception from mmu_translate
  target/s390x: Return exception from translate_pages
  target/s390x: Remove fail variable from s390_cpu_tlb_fill
  target/s390x: Simplify helper_lra
  target/s390x: Rely on unwinding in s390_cpu_tlb_fill
  target/s390x: Rely on unwinding in s390_cpu_virt_mem_rw
  target/s390x: Remove ILEN_AUTO
  target/s390x: Remove ilen argument from trigger_access_exception
  target/s390x: Remove ilen argument from trigger_pgm_exception
  target/s390x: Remove ILEN_UNWIND

 hw/s390x/s390-pci-inst.c     |  58 ++---
 hw/s390x/s390-virtio-ccw.c   |   2 +
 include/exec/exec-all.h      |  17 ++
 target/s390x/cc_helper.c     |   4 +-
 target/s390x/cpu.h           |  86 +++++--
 target/s390x/crypto_helper.c |   7 +-
 target/s390x/diag.c          |  14 +-
 target/s390x/excp_helper.c   |  58 +++--
 target/s390x/fpu_helper.c    |   6 +-
 target/s390x/gen-features.c  |  11 +-
 target/s390x/helper.c        |   7 +-
 target/s390x/int_helper.c    |  15 +-
 target/s390x/internal.h      |   6 +-
 target/s390x/interrupt.c     |   9 +-
 target/s390x/ioinst.c        |  40 ++--
 target/s390x/mem_helper.c    |  84 +++----
 target/s390x/misc_helper.c   |  27 +--
 target/s390x/mmu_helper.c    | 429 ++++++++++++++++-------------------
 target/s390x/tcg-stub.c      |   4 +-
 target/s390x/tcg_s390x.h     |   4 +-
 target/s390x/translate.c     |  20 +-
 21 files changed, 475 insertions(+), 433 deletions(-)

-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 01/31] s390x/mmu: Drop debug logging from MMU code
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 02/31] s390x/mmu: Move DAT protection handling out of mmu_translate_asce() David Hildenbrand
                   ` (31 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Let's get it out of the way to make some further refactorings easier.
Personally, I've never used these debug statements at all. And if I had
to debug issues, I used plain GDB instead (debug prints are just way too
much noise in the MMU). We might want to introduce tracing at some point
instead, so we can able selected events on demand.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 51 ---------------------------------------
 1 file changed, 51 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 7e6b0d0508..6a7ad33c4d 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -28,31 +28,6 @@
 #include "hw/hw.h"
 #include "hw/s390x/storage-keys.h"
 
-/* #define DEBUG_S390 */
-/* #define DEBUG_S390_PTE */
-/* #define DEBUG_S390_STDOUT */
-
-#ifdef DEBUG_S390
-#ifdef DEBUG_S390_STDOUT
-#define DPRINTF(fmt, ...) \
-    do { fprintf(stderr, fmt, ## __VA_ARGS__); \
-         if (qemu_log_separate()) qemu_log(fmt, ##__VA_ARGS__); } while (0)
-#else
-#define DPRINTF(fmt, ...) \
-    do { qemu_log(fmt, ## __VA_ARGS__); } while (0)
-#endif
-#else
-#define DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
-#ifdef DEBUG_S390_PTE
-#define PTE_DPRINTF DPRINTF
-#else
-#define PTE_DPRINTF(fmt, ...) \
-    do { } while (0)
-#endif
-
 /* Fetch/store bits in the translation exception code: */
 #define FS_READ  0x800
 #define FS_WRITE 0x400
@@ -80,8 +55,6 @@ static void trigger_prot_fault(CPUS390XState *env, target_ulong vaddr,
 
     tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | 4 | asc >> 46;
 
-    DPRINTF("%s: trans_exc_code=%016" PRIx64 "\n", __func__, tec);
-
     if (!exc) {
         return;
     }
@@ -97,8 +70,6 @@ static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr,
 
     tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | asc >> 46;
 
-    DPRINTF("%s: trans_exc_code=%016" PRIx64 "\n", __func__, tec);
-
     if (!exc) {
         return;
     }
@@ -162,7 +133,6 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
                              target_ulong *raddr, int *flags, int rw, bool exc)
 {
     if (pt_entry & PAGE_INVALID) {
-        DPRINTF("%s: PTE=0x%" PRIx64 " invalid\n", __func__, pt_entry);
         trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw, exc);
         return -1;
     }
@@ -175,9 +145,6 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
     }
 
     *raddr = pt_entry & ASCE_ORIGIN;
-
-    PTE_DPRINTF("%s: PTE=0x%" PRIx64 "\n", __func__, pt_entry);
-
     return 0;
 }
 
@@ -197,7 +164,6 @@ static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
     if ((st_entry & SEGMENT_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
         /* Decode EDAT1 segment frame absolute address (1MB page) */
         *raddr = (st_entry & 0xfffffffffff00000ULL) | (vaddr & 0xfffff);
-        PTE_DPRINTF("%s: SEG=0x%" PRIx64 "\n", __func__, st_entry);
         return 0;
     }
 
@@ -205,8 +171,6 @@ static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
     origin = st_entry & SEGMENT_ENTRY_ORIGIN;
     offs  = (vaddr & VADDR_PX) >> 9;
     pt_entry = ldq_phys(cs->as, origin + offs);
-    PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
-                __func__, origin, offs, pt_entry);
     return mmu_translate_pte(env, vaddr, asc, pt_entry, raddr, flags, rw, exc);
 }
 
@@ -223,17 +187,12 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
         PGM_REG_SEC_TRANS, PGM_REG_FIRST_TRANS
     };
 
-    PTE_DPRINTF("%s: 0x%" PRIx64 "\n", __func__, entry);
-
     origin = entry & REGION_ENTRY_ORIGIN;
     offs = (vaddr >> (17 + 11 * level / 4)) & 0x3ff8;
 
     new_entry = ldq_phys(cs->as, origin + offs);
-    PTE_DPRINTF("%s: 0x%" PRIx64 " + 0x%" PRIx64 " => 0x%016" PRIx64 "\n",
-                __func__, origin, offs, new_entry);
 
     if ((new_entry & REGION_ENTRY_INV) != 0) {
-        DPRINTF("%s: invalid region\n", __func__);
         trigger_page_fault(env, vaddr, pchks[level / 4], asc, rw, exc);
         return -1;
     }
@@ -252,7 +211,6 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
     offs = (vaddr >> (28 + 11 * (level - 4) / 4)) & 3;
     if (offs < ((new_entry & REGION_ENTRY_TF) >> 6)
         || offs > (new_entry & REGION_ENTRY_LENGTH)) {
-        DPRINTF("%s: invalid offset or len (%lx)\n", __func__, new_entry);
         trigger_page_fault(env, vaddr, pchks[level / 4 - 1], asc, rw, exc);
         return -1;
     }
@@ -289,8 +247,6 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         break;
     case ASCE_TYPE_REGION2:
         if (vaddr & 0xffe0000000000000ULL) {
-            DPRINTF("%s: vaddr doesn't fit 0x%16" PRIx64
-                    " 0xffe0000000000000ULL\n", __func__, vaddr);
             trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
             return -1;
         }
@@ -301,8 +257,6 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         break;
     case ASCE_TYPE_REGION3:
         if (vaddr & 0xfffffc0000000000ULL) {
-            DPRINTF("%s: vaddr doesn't fit 0x%16" PRIx64
-                    " 0xfffffc0000000000ULL\n", __func__, vaddr);
             trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
             return -1;
         }
@@ -313,8 +267,6 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         break;
     case ASCE_TYPE_SEGMENT:
         if (vaddr & 0xffffffff80000000ULL) {
-            DPRINTF("%s: vaddr doesn't fit 0x%16" PRIx64
-                    " 0xffffffff80000000ULL\n", __func__, vaddr);
             trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
             return -1;
         }
@@ -449,15 +401,12 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 
     switch (asc) {
     case PSW_ASC_PRIMARY:
-        PTE_DPRINTF("%s: asc=primary\n", __func__);
         asce = env->cregs[1];
         break;
     case PSW_ASC_HOME:
-        PTE_DPRINTF("%s: asc=home\n", __func__);
         asce = env->cregs[13];
         break;
     case PSW_ASC_SECONDARY:
-        PTE_DPRINTF("%s: asc=secondary\n", __func__);
         asce = env->cregs[7];
         break;
     case PSW_ASC_ACCREG:
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 02/31] s390x/mmu: Move DAT protection handling out of mmu_translate_asce()
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
  2019-10-10 11:33 ` [PULL 01/31] s390x/mmu: Drop debug logging from MMU code David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 03/31] s390x/mmu: Inject DAT exceptions from a single place David Hildenbrand
                   ` (30 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

We'll reuse the ilen and tec definitions in mmu_translate
soon also for all other DAT exceptions we inject. Move it to the caller,
where we can later pair it up with other protection checks, like IEP.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 39 ++++++++++++++++-----------------------
 1 file changed, 16 insertions(+), 23 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 6a7ad33c4d..847fb240fb 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -48,20 +48,6 @@ static void trigger_access_exception(CPUS390XState *env, uint32_t type,
     }
 }
 
-static void trigger_prot_fault(CPUS390XState *env, target_ulong vaddr,
-                               uint64_t asc, int rw, bool exc)
-{
-    uint64_t tec;
-
-    tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | 4 | asc >> 46;
-
-    if (!exc) {
-        return;
-    }
-
-    trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, tec);
-}
-
 static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr,
                                uint32_t type, uint64_t asc, int rw, bool exc)
 {
@@ -229,7 +215,6 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
                               int *flags, int rw, bool exc)
 {
     int level;
-    int r;
 
     if (asce & ASCE_REAL_SPACE) {
         /* direct mapping */
@@ -277,14 +262,8 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         break;
     }
 
-    r = mmu_translate_region(env, vaddr, asc, asce, level, raddr, flags, rw,
-                             exc);
-    if (!r && rw == MMU_DATA_STORE && !(*flags & PAGE_WRITE)) {
-        trigger_prot_fault(env, vaddr, asc, rw, exc);
-        return -1;
-    }
-
-    return r;
+    return mmu_translate_region(env, vaddr, asc, asce, level, raddr, flags, rw,
+                                exc);
 }
 
 static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
@@ -369,6 +348,10 @@ static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
                   target_ulong *raddr, int *flags, bool exc)
 {
+    /* Code accesses have an undefined ilc, let's use 2 bytes. */
+    const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
+    uint64_t tec = (vaddr & TARGET_PAGE_MASK) | (asc >> 46) |
+                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     uint64_t asce;
     int r;
 
@@ -421,6 +404,16 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         return r;
     }
 
+    /* check for DAT protection */
+    if (unlikely(rw == MMU_DATA_STORE && !(*flags & PAGE_WRITE))) {
+        if (exc) {
+            /* DAT sets bit 61 only */
+            tec |= 0x4;
+            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
+        }
+        return -1;
+    }
+
 nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 03/31] s390x/mmu: Inject DAT exceptions from a single place
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
  2019-10-10 11:33 ` [PULL 01/31] s390x/mmu: Drop debug logging from MMU code David Hildenbrand
  2019-10-10 11:33 ` [PULL 02/31] s390x/mmu: Move DAT protection handling out of mmu_translate_asce() David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 04/31] s390x/mmu: Inject PGM_ADDRESSING on bogus table addresses David Hildenbrand
                   ` (29 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Let's return the PGM from the translation functions on error and inject
based on that.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 63 +++++++++++----------------------------
 1 file changed, 17 insertions(+), 46 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 847fb240fb..f6ae444655 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -48,26 +48,6 @@ static void trigger_access_exception(CPUS390XState *env, uint32_t type,
     }
 }
 
-static void trigger_page_fault(CPUS390XState *env, target_ulong vaddr,
-                               uint32_t type, uint64_t asc, int rw, bool exc)
-{
-    int ilen = ILEN_AUTO;
-    uint64_t tec;
-
-    tec = vaddr | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ) | asc >> 46;
-
-    if (!exc) {
-        return;
-    }
-
-    /* Code accesses have an undefined ilc.  */
-    if (rw == MMU_INST_FETCH) {
-        ilen = 2;
-    }
-
-    trigger_access_exception(env, type, ilen, tec);
-}
-
 /* check whether the address would be proteted by Low-Address Protection */
 static bool is_low_address(uint64_t addr)
 {
@@ -119,12 +99,10 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
                              target_ulong *raddr, int *flags, int rw, bool exc)
 {
     if (pt_entry & PAGE_INVALID) {
-        trigger_page_fault(env, vaddr, PGM_PAGE_TRANS, asc, rw, exc);
-        return -1;
+        return PGM_PAGE_TRANS;
     }
     if (pt_entry & PAGE_RES0) {
-        trigger_page_fault(env, vaddr, PGM_TRANS_SPEC, asc, rw, exc);
-        return -1;
+        return PGM_TRANS_SPEC;
     }
     if (pt_entry & PAGE_RO) {
         *flags &= ~PAGE_WRITE;
@@ -179,13 +157,11 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
     new_entry = ldq_phys(cs->as, origin + offs);
 
     if ((new_entry & REGION_ENTRY_INV) != 0) {
-        trigger_page_fault(env, vaddr, pchks[level / 4], asc, rw, exc);
-        return -1;
+        return pchks[level / 4];
     }
 
     if ((new_entry & REGION_ENTRY_TYPE_MASK) != level) {
-        trigger_page_fault(env, vaddr, PGM_TRANS_SPEC, asc, rw, exc);
-        return -1;
+        return PGM_TRANS_SPEC;
     }
 
     if (level == ASCE_TYPE_SEGMENT) {
@@ -197,8 +173,7 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
     offs = (vaddr >> (28 + 11 * (level - 4) / 4)) & 3;
     if (offs < ((new_entry & REGION_ENTRY_TF) >> 6)
         || offs > (new_entry & REGION_ENTRY_LENGTH)) {
-        trigger_page_fault(env, vaddr, pchks[level / 4 - 1], asc, rw, exc);
-        return -1;
+        return pchks[level / 4 - 1];
     }
 
     if ((env->cregs[0] & CR0_EDAT) && (new_entry & REGION_ENTRY_RO)) {
@@ -226,38 +201,31 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     switch (level) {
     case ASCE_TYPE_REGION1:
         if ((vaddr >> 62) > (asce & ASCE_TABLE_LENGTH)) {
-            trigger_page_fault(env, vaddr, PGM_REG_FIRST_TRANS, asc, rw, exc);
-            return -1;
+            return PGM_REG_FIRST_TRANS;
         }
         break;
     case ASCE_TYPE_REGION2:
         if (vaddr & 0xffe0000000000000ULL) {
-            trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
-            return -1;
+            return PGM_ASCE_TYPE;
         }
         if ((vaddr >> 51 & 3) > (asce & ASCE_TABLE_LENGTH)) {
-            trigger_page_fault(env, vaddr, PGM_REG_SEC_TRANS, asc, rw, exc);
-            return -1;
+            return PGM_REG_SEC_TRANS;
         }
         break;
     case ASCE_TYPE_REGION3:
         if (vaddr & 0xfffffc0000000000ULL) {
-            trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
-            return -1;
+            return PGM_ASCE_TYPE;
         }
         if ((vaddr >> 40 & 3) > (asce & ASCE_TABLE_LENGTH)) {
-            trigger_page_fault(env, vaddr, PGM_REG_THIRD_TRANS, asc, rw, exc);
-            return -1;
+            return PGM_REG_THIRD_TRANS;
         }
         break;
     case ASCE_TYPE_SEGMENT:
         if (vaddr & 0xffffffff80000000ULL) {
-            trigger_page_fault(env, vaddr, PGM_ASCE_TYPE, asc, rw, exc);
-            return -1;
+            return PGM_ASCE_TYPE;
         }
         if ((vaddr >> 29 & 3) > (asce & ASCE_TABLE_LENGTH)) {
-            trigger_page_fault(env, vaddr, PGM_SEGMENT_TRANS, asc, rw, exc);
-            return -1;
+            return PGM_SEGMENT_TRANS;
         }
         break;
     }
@@ -400,8 +368,11 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
 
     /* perform the DAT translation */
     r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
-    if (r) {
-        return r;
+    if (unlikely(r)) {
+        if (exc) {
+            trigger_access_exception(env, r, ilen, tec);
+        }
+        return -1;
     }
 
     /* check for DAT protection */
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 04/31] s390x/mmu: Inject PGM_ADDRESSING on bogus table addresses
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (2 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 03/31] s390x/mmu: Inject DAT exceptions from a single place David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 05/31] s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte() David Hildenbrand
                   ` (28 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Let's document how it works and inject PGM_ADDRESSING if reading of
table entries fails.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index f6ae444655..71dee0a5d9 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -93,6 +93,27 @@ target_ulong mmu_real2abs(CPUS390XState *env, target_ulong raddr)
     return raddr;
 }
 
+static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
+                                    uint64_t *entry)
+{
+    CPUState *cs = env_cpu(env);
+
+    /*
+     * According to the PoP, these table addresses are "unpredictably real
+     * or absolute". Also, "it is unpredictable whether the address wraps
+     * or an addressing exception is recognized".
+     *
+     * We treat them as absolute addresses and don't wrap them.
+     */
+    if (unlikely(address_space_read(cs->as, gaddr, MEMTXATTRS_UNSPECIFIED,
+                                    (uint8_t *)entry, sizeof(*entry)) !=
+                 MEMTX_OK)) {
+        return false;
+    }
+    *entry = be64_to_cpu(*entry);
+    return true;
+}
+
 /* Decode page table entry (normal 4KB page) */
 static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
                              uint64_t asc, uint64_t pt_entry,
@@ -118,7 +139,6 @@ static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
                                  target_ulong *raddr, int *flags, int rw,
                                  bool exc)
 {
-    CPUState *cs = env_cpu(env);
     uint64_t origin, offs, pt_entry;
 
     if (st_entry & SEGMENT_ENTRY_RO) {
@@ -134,7 +154,9 @@ static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
     /* Look up 4KB page entry */
     origin = st_entry & SEGMENT_ENTRY_ORIGIN;
     offs  = (vaddr & VADDR_PX) >> 9;
-    pt_entry = ldq_phys(cs->as, origin + offs);
+    if (!read_table_entry(env, origin + offs, &pt_entry)) {
+        return PGM_ADDRESSING;
+    }
     return mmu_translate_pte(env, vaddr, asc, pt_entry, raddr, flags, rw, exc);
 }
 
@@ -144,7 +166,6 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
                                 target_ulong *raddr, int *flags, int rw,
                                 bool exc)
 {
-    CPUState *cs = env_cpu(env);
     uint64_t origin, offs, new_entry;
     const int pchks[4] = {
         PGM_SEGMENT_TRANS, PGM_REG_THIRD_TRANS,
@@ -154,7 +175,9 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
     origin = entry & REGION_ENTRY_ORIGIN;
     offs = (vaddr >> (17 + 11 * level / 4)) & 0x3ff8;
 
-    new_entry = ldq_phys(cs->as, origin + offs);
+    if (!read_table_entry(env, origin + offs, &new_entry)) {
+        return PGM_ADDRESSING;
+    }
 
     if ((new_entry & REGION_ENTRY_INV) != 0) {
         return pchks[level / 4];
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 05/31] s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte()
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (3 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 04/31] s390x/mmu: Inject PGM_ADDRESSING on bogus table addresses David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 06/31] s390x/mmu: DAT table definition overhaul David Hildenbrand
                   ` (27 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

While ASCE_ORIGIN is not wrong, it is certainly confusing. We want a
page frame address.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 71dee0a5d9..aaf5b23513 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -129,7 +129,7 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
         *flags &= ~PAGE_WRITE;
     }
 
-    *raddr = pt_entry & ASCE_ORIGIN;
+    *raddr = pt_entry & TARGET_PAGE_MASK;
     return 0;
 }
 
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 06/31] s390x/mmu: DAT table definition overhaul
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (4 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 05/31] s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte() David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 07/31] s390x/mmu: Convert to non-recursive page table walk David Hildenbrand
                   ` (26 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Let's use consistent names for the region/section/page table entries and
for the macros to extract relevant parts from virtual address. Make them
match the definitions in the PoP - e.g., how the relevant bits are actually
called.

Introduce defines for all bits declared in the PoP. This will come in
handy in follow-up patches.

Add a note where additional information about s390x and the used
definitions can be found.

Acked-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h        | 78 +++++++++++++++++++++++++++++----------
 target/s390x/mem_helper.c | 12 +++---
 target/s390x/mmu_helper.c | 37 ++++++++++---------
 3 files changed, 84 insertions(+), 43 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 163dae13d7..2db54884b8 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -1,6 +1,10 @@
 /*
  * S/390 virtual CPU header
  *
+ * For details on the s390x architecture and used definitions (e.g.,
+ * PSW, PER and DAT (Dynamic Address Translation)), please refer to
+ * the "z/Architecture Principles of Operations" - a.k.a. PoP.
+ *
  *  Copyright (c) 2009 Ulrich Hecht
  *  Copyright IBM Corp. 2012, 2018
  *
@@ -558,26 +562,60 @@ QEMU_BUILD_BUG_ON(sizeof(SysIB) != 4096);
 #define ASCE_TYPE_SEGMENT     0x00        /* segment table type               */
 #define ASCE_TABLE_LENGTH     0x03        /* region table length              */
 
-#define REGION_ENTRY_ORIGIN   (~0xfffULL) /* region/segment table origin    */
-#define REGION_ENTRY_RO       0x200       /* region/segment protection bit  */
-#define REGION_ENTRY_TF       0xc0        /* region/segment table offset    */
-#define REGION_ENTRY_INV      0x20        /* invalid region table entry     */
-#define REGION_ENTRY_TYPE_MASK 0x0c       /* region/segment table type mask */
-#define REGION_ENTRY_TYPE_R1  0x0c        /* region first table type        */
-#define REGION_ENTRY_TYPE_R2  0x08        /* region second table type       */
-#define REGION_ENTRY_TYPE_R3  0x04        /* region third table type        */
-#define REGION_ENTRY_LENGTH   0x03        /* region third length            */
-
-#define SEGMENT_ENTRY_ORIGIN  (~0x7ffULL) /* segment table origin        */
-#define SEGMENT_ENTRY_FC      0x400       /* format control              */
-#define SEGMENT_ENTRY_RO      0x200       /* page protection bit         */
-#define SEGMENT_ENTRY_INV     0x20        /* invalid segment table entry */
-
-#define VADDR_PX              0xff000     /* page index bits   */
-
-#define PAGE_RO               0x200       /* HW read-only bit  */
-#define PAGE_INVALID          0x400       /* HW invalid bit    */
-#define PAGE_RES0             0x800       /* bit must be zero  */
+#define REGION_ENTRY_ORIGIN         0xfffffffffffff000ULL
+#define REGION_ENTRY_P              0x0000000000000200ULL
+#define REGION_ENTRY_TF             0x00000000000000c0ULL
+#define REGION_ENTRY_I              0x0000000000000020ULL
+#define REGION_ENTRY_TT             0x000000000000000cULL
+#define REGION_ENTRY_TL             0x0000000000000003ULL
+
+#define REGION_ENTRY_TT_REGION1     0x000000000000000cULL
+#define REGION_ENTRY_TT_REGION2     0x0000000000000008ULL
+#define REGION_ENTRY_TT_REGION3     0x0000000000000004ULL
+
+#define REGION3_ENTRY_RFAA          0xffffffff80000000ULL
+#define REGION3_ENTRY_AV            0x0000000000010000ULL
+#define REGION3_ENTRY_ACC           0x000000000000f000ULL
+#define REGION3_ENTRY_F             0x0000000000000800ULL
+#define REGION3_ENTRY_FC            0x0000000000000400ULL
+#define REGION3_ENTRY_IEP           0x0000000000000100ULL
+#define REGION3_ENTRY_CR            0x0000000000000010ULL
+
+#define SEGMENT_ENTRY_ORIGIN        0xfffffffffffff800ULL
+#define SEGMENT_ENTRY_SFAA          0xfffffffffff00000ULL
+#define SEGMENT_ENTRY_AV            0x0000000000010000ULL
+#define SEGMENT_ENTRY_ACC           0x000000000000f000ULL
+#define SEGMENT_ENTRY_F             0x0000000000000800ULL
+#define SEGMENT_ENTRY_FC            0x0000000000000400ULL
+#define SEGMENT_ENTRY_P             0x0000000000000200ULL
+#define SEGMENT_ENTRY_IEP           0x0000000000000100ULL
+#define SEGMENT_ENTRY_I             0x0000000000000020ULL
+#define SEGMENT_ENTRY_CS            0x0000000000000010ULL
+#define SEGMENT_ENTRY_TT            0x000000000000000cULL
+
+#define SEGMENT_ENTRY_TT_SEGMENT    0x0000000000000000ULL
+
+#define PAGE_ENTRY_0                0x0000000000000800ULL
+#define PAGE_ENTRY_I                0x0000000000000400ULL
+#define PAGE_ENTRY_P                0x0000000000000200ULL
+#define PAGE_ENTRY_IEP              0x0000000000000100ULL
+
+#define VADDR_REGION1_TX_MASK       0xffe0000000000000ULL
+#define VADDR_REGION2_TX_MASK       0x001ffc0000000000ULL
+#define VADDR_REGION3_TX_MASK       0x000003ff80000000ULL
+#define VADDR_SEGMENT_TX_MASK       0x000000007ff00000ULL
+#define VADDR_PAGE_TX_MASK          0x00000000000ff000ULL
+
+#define VADDR_REGION1_TX(vaddr)     (((vaddr) & VADDR_REGION1_TX_MASK) >> 53)
+#define VADDR_REGION2_TX(vaddr)     (((vaddr) & VADDR_REGION2_TX_MASK) >> 42)
+#define VADDR_REGION3_TX(vaddr)     (((vaddr) & VADDR_REGION3_TX_MASK) >> 31)
+#define VADDR_SEGMENT_TX(vaddr)     (((vaddr) & VADDR_SEGMENT_TX_MASK) >> 20)
+#define VADDR_PAGE_TX(vaddr)        (((vaddr) & VADDR_PAGE_TX_MASK) >> 12)
+
+#define VADDR_REGION1_TL(vaddr)     (((vaddr) & 0xc000000000000000ULL) >> 62)
+#define VADDR_REGION2_TL(vaddr)     (((vaddr) & 0x0018000000000000ULL) >> 51)
+#define VADDR_REGION3_TL(vaddr)     (((vaddr) & 0x0000030000000000ULL) >> 40)
+#define VADDR_SEGMENT_TL(vaddr)     (((vaddr) & 0x0000000060000000ULL) >> 29)
 
 #define SK_C                    (0x1 << 1)
 #define SK_R                    (0x1 << 2)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 44e535856d..08c5cc6a99 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2252,9 +2252,9 @@ void HELPER(idte)(CPUS390XState *env, uint64_t r1, uint64_t r2, uint32_t m4)
             /* addresses are not wrapped in 24/31bit mode but table index is */
             raddr = table + ((index + i) & 0x7ff) * sizeof(entry);
             entry = cpu_ldq_real_ra(env, raddr, ra);
-            if (!(entry & REGION_ENTRY_INV)) {
+            if (!(entry & REGION_ENTRY_I)) {
                 /* we are allowed to not store if already invalid */
-                entry |= REGION_ENTRY_INV;
+                entry |= REGION_ENTRY_I;
                 cpu_stq_real_ra(env, raddr, entry, ra);
             }
         }
@@ -2279,17 +2279,17 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr,
 
     /* Compute the page table entry address */
     pte_addr = (pto & SEGMENT_ENTRY_ORIGIN);
-    pte_addr += (vaddr & VADDR_PX) >> 9;
+    pte_addr += VADDR_PAGE_TX(vaddr) * 8;
 
     /* Mark the page table entry as invalid */
     pte = cpu_ldq_real_ra(env, pte_addr, ra);
-    pte |= PAGE_INVALID;
+    pte |= PAGE_ENTRY_I;
     cpu_stq_real_ra(env, pte_addr, pte, ra);
 
     /* XXX we exploit the fact that Linux passes the exact virtual
        address here - it's not obliged to! */
     if (m4 & 1) {
-        if (vaddr & ~VADDR_PX) {
+        if (vaddr & ~VADDR_PAGE_TX_MASK) {
             tlb_flush_page(cs, page);
             /* XXX 31-bit hack */
             tlb_flush_page(cs, page ^ 0x80000000);
@@ -2298,7 +2298,7 @@ void HELPER(ipte)(CPUS390XState *env, uint64_t pto, uint64_t vaddr,
             tlb_flush(cs);
         }
     } else {
-        if (vaddr & ~VADDR_PX) {
+        if (vaddr & ~VADDR_PAGE_TX_MASK) {
             tlb_flush_page_all_cpus_synced(cs, page);
             /* XXX 31-bit hack */
             tlb_flush_page_all_cpus_synced(cs, page ^ 0x80000000);
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index aaf5b23513..a114fb1628 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -119,13 +119,13 @@ static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
                              uint64_t asc, uint64_t pt_entry,
                              target_ulong *raddr, int *flags, int rw, bool exc)
 {
-    if (pt_entry & PAGE_INVALID) {
+    if (pt_entry & PAGE_ENTRY_I) {
         return PGM_PAGE_TRANS;
     }
-    if (pt_entry & PAGE_RES0) {
+    if (pt_entry & PAGE_ENTRY_0) {
         return PGM_TRANS_SPEC;
     }
-    if (pt_entry & PAGE_RO) {
+    if (pt_entry & PAGE_ENTRY_P) {
         *flags &= ~PAGE_WRITE;
     }
 
@@ -141,19 +141,20 @@ static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
 {
     uint64_t origin, offs, pt_entry;
 
-    if (st_entry & SEGMENT_ENTRY_RO) {
+    if (st_entry & SEGMENT_ENTRY_P) {
         *flags &= ~PAGE_WRITE;
     }
 
     if ((st_entry & SEGMENT_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
         /* Decode EDAT1 segment frame absolute address (1MB page) */
-        *raddr = (st_entry & 0xfffffffffff00000ULL) | (vaddr & 0xfffff);
+        *raddr = (st_entry & SEGMENT_ENTRY_SFAA) |
+                 (vaddr & ~SEGMENT_ENTRY_SFAA);
         return 0;
     }
 
     /* Look up 4KB page entry */
     origin = st_entry & SEGMENT_ENTRY_ORIGIN;
-    offs  = (vaddr & VADDR_PX) >> 9;
+    offs = VADDR_PAGE_TX(vaddr) * 8;
     if (!read_table_entry(env, origin + offs, &pt_entry)) {
         return PGM_ADDRESSING;
     }
@@ -179,11 +180,11 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
         return PGM_ADDRESSING;
     }
 
-    if ((new_entry & REGION_ENTRY_INV) != 0) {
+    if (new_entry & REGION_ENTRY_I) {
         return pchks[level / 4];
     }
 
-    if ((new_entry & REGION_ENTRY_TYPE_MASK) != level) {
+    if ((new_entry & REGION_ENTRY_TT) != level) {
         return PGM_TRANS_SPEC;
     }
 
@@ -195,11 +196,11 @@ static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
     /* Check region table offset and length */
     offs = (vaddr >> (28 + 11 * (level - 4) / 4)) & 3;
     if (offs < ((new_entry & REGION_ENTRY_TF) >> 6)
-        || offs > (new_entry & REGION_ENTRY_LENGTH)) {
+        || offs > (new_entry & REGION_ENTRY_TL)) {
         return pchks[level / 4 - 1];
     }
 
-    if ((env->cregs[0] & CR0_EDAT) && (new_entry & REGION_ENTRY_RO)) {
+    if ((env->cregs[0] & CR0_EDAT) && (new_entry & REGION_ENTRY_P)) {
         *flags &= ~PAGE_WRITE;
     }
 
@@ -212,6 +213,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
                               uint64_t asc, uint64_t asce, target_ulong *raddr,
                               int *flags, int rw, bool exc)
 {
+    const int asce_tl = asce & ASCE_TABLE_LENGTH;
     int level;
 
     if (asce & ASCE_REAL_SPACE) {
@@ -223,31 +225,32 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     level = asce & ASCE_TYPE_MASK;
     switch (level) {
     case ASCE_TYPE_REGION1:
-        if ((vaddr >> 62) > (asce & ASCE_TABLE_LENGTH)) {
+        if (VADDR_REGION1_TL(vaddr) > asce_tl) {
             return PGM_REG_FIRST_TRANS;
         }
         break;
     case ASCE_TYPE_REGION2:
-        if (vaddr & 0xffe0000000000000ULL) {
+        if (VADDR_REGION1_TX(vaddr)) {
             return PGM_ASCE_TYPE;
         }
-        if ((vaddr >> 51 & 3) > (asce & ASCE_TABLE_LENGTH)) {
+        if (VADDR_REGION2_TL(vaddr) > asce_tl) {
             return PGM_REG_SEC_TRANS;
         }
         break;
     case ASCE_TYPE_REGION3:
-        if (vaddr & 0xfffffc0000000000ULL) {
+        if (VADDR_REGION1_TX(vaddr) || VADDR_REGION2_TX(vaddr)) {
             return PGM_ASCE_TYPE;
         }
-        if ((vaddr >> 40 & 3) > (asce & ASCE_TABLE_LENGTH)) {
+        if (VADDR_REGION3_TL(vaddr) > asce_tl) {
             return PGM_REG_THIRD_TRANS;
         }
         break;
     case ASCE_TYPE_SEGMENT:
-        if (vaddr & 0xffffffff80000000ULL) {
+        if (VADDR_REGION1_TX(vaddr) || VADDR_REGION2_TX(vaddr) ||
+            VADDR_REGION3_TX(vaddr)) {
             return PGM_ASCE_TYPE;
         }
-        if ((vaddr >> 29 & 3) > (asce & ASCE_TABLE_LENGTH)) {
+        if (VADDR_SEGMENT_TL(vaddr) > asce_tl) {
             return PGM_SEGMENT_TRANS;
         }
         break;
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 07/31] s390x/mmu: Convert to non-recursive page table walk
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (5 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 06/31] s390x/mmu: DAT table definition overhaul David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 08/31] s390x/mmu: Add EDAT2 translation support David Hildenbrand
                   ` (25 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

A non-recursive implementation allows to make better use of the
branch predictor, avoids function calls, and makes the implementation of
new features only for a subset of region table levels easier.

We can now directly compare our implementation to the KVM gaccess
implementation in arch/s390/kvm/gaccess.c:guest_translate().

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 208 ++++++++++++++++++++------------------
 1 file changed, 108 insertions(+), 100 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index a114fb1628..dc33c63b1d 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -114,107 +114,16 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
     return true;
 }
 
-/* Decode page table entry (normal 4KB page) */
-static int mmu_translate_pte(CPUS390XState *env, target_ulong vaddr,
-                             uint64_t asc, uint64_t pt_entry,
-                             target_ulong *raddr, int *flags, int rw, bool exc)
-{
-    if (pt_entry & PAGE_ENTRY_I) {
-        return PGM_PAGE_TRANS;
-    }
-    if (pt_entry & PAGE_ENTRY_0) {
-        return PGM_TRANS_SPEC;
-    }
-    if (pt_entry & PAGE_ENTRY_P) {
-        *flags &= ~PAGE_WRITE;
-    }
-
-    *raddr = pt_entry & TARGET_PAGE_MASK;
-    return 0;
-}
-
-/* Decode segment table entry */
-static int mmu_translate_segment(CPUS390XState *env, target_ulong vaddr,
-                                 uint64_t asc, uint64_t st_entry,
-                                 target_ulong *raddr, int *flags, int rw,
-                                 bool exc)
-{
-    uint64_t origin, offs, pt_entry;
-
-    if (st_entry & SEGMENT_ENTRY_P) {
-        *flags &= ~PAGE_WRITE;
-    }
-
-    if ((st_entry & SEGMENT_ENTRY_FC) && (env->cregs[0] & CR0_EDAT)) {
-        /* Decode EDAT1 segment frame absolute address (1MB page) */
-        *raddr = (st_entry & SEGMENT_ENTRY_SFAA) |
-                 (vaddr & ~SEGMENT_ENTRY_SFAA);
-        return 0;
-    }
-
-    /* Look up 4KB page entry */
-    origin = st_entry & SEGMENT_ENTRY_ORIGIN;
-    offs = VADDR_PAGE_TX(vaddr) * 8;
-    if (!read_table_entry(env, origin + offs, &pt_entry)) {
-        return PGM_ADDRESSING;
-    }
-    return mmu_translate_pte(env, vaddr, asc, pt_entry, raddr, flags, rw, exc);
-}
-
-/* Decode region table entries */
-static int mmu_translate_region(CPUS390XState *env, target_ulong vaddr,
-                                uint64_t asc, uint64_t entry, int level,
-                                target_ulong *raddr, int *flags, int rw,
-                                bool exc)
-{
-    uint64_t origin, offs, new_entry;
-    const int pchks[4] = {
-        PGM_SEGMENT_TRANS, PGM_REG_THIRD_TRANS,
-        PGM_REG_SEC_TRANS, PGM_REG_FIRST_TRANS
-    };
-
-    origin = entry & REGION_ENTRY_ORIGIN;
-    offs = (vaddr >> (17 + 11 * level / 4)) & 0x3ff8;
-
-    if (!read_table_entry(env, origin + offs, &new_entry)) {
-        return PGM_ADDRESSING;
-    }
-
-    if (new_entry & REGION_ENTRY_I) {
-        return pchks[level / 4];
-    }
-
-    if ((new_entry & REGION_ENTRY_TT) != level) {
-        return PGM_TRANS_SPEC;
-    }
-
-    if (level == ASCE_TYPE_SEGMENT) {
-        return mmu_translate_segment(env, vaddr, asc, new_entry, raddr, flags,
-                                     rw, exc);
-    }
-
-    /* Check region table offset and length */
-    offs = (vaddr >> (28 + 11 * (level - 4) / 4)) & 3;
-    if (offs < ((new_entry & REGION_ENTRY_TF) >> 6)
-        || offs > (new_entry & REGION_ENTRY_TL)) {
-        return pchks[level / 4 - 1];
-    }
-
-    if ((env->cregs[0] & CR0_EDAT) && (new_entry & REGION_ENTRY_P)) {
-        *flags &= ~PAGE_WRITE;
-    }
-
-    /* yet another region */
-    return mmu_translate_region(env, vaddr, asc, new_entry, level - 4,
-                                raddr, flags, rw, exc);
-}
-
 static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
                               uint64_t asc, uint64_t asce, target_ulong *raddr,
                               int *flags, int rw, bool exc)
 {
+    const bool edat1 = (env->cregs[0] & CR0_EDAT) &&
+                       s390_has_feat(S390_FEAT_EDAT);
     const int asce_tl = asce & ASCE_TABLE_LENGTH;
-    int level;
+    const int asce_p = asce & ASCE_PRIVATE_SPACE;
+    hwaddr gaddr = asce & ASCE_ORIGIN;
+    uint64_t entry;
 
     if (asce & ASCE_REAL_SPACE) {
         /* direct mapping */
@@ -222,12 +131,12 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         return 0;
     }
 
-    level = asce & ASCE_TYPE_MASK;
-    switch (level) {
+    switch (asce & ASCE_TYPE_MASK) {
     case ASCE_TYPE_REGION1:
         if (VADDR_REGION1_TL(vaddr) > asce_tl) {
             return PGM_REG_FIRST_TRANS;
         }
+        gaddr += VADDR_REGION1_TX(vaddr) * 8;
         break;
     case ASCE_TYPE_REGION2:
         if (VADDR_REGION1_TX(vaddr)) {
@@ -236,6 +145,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         if (VADDR_REGION2_TL(vaddr) > asce_tl) {
             return PGM_REG_SEC_TRANS;
         }
+        gaddr += VADDR_REGION2_TX(vaddr) * 8;
         break;
     case ASCE_TYPE_REGION3:
         if (VADDR_REGION1_TX(vaddr) || VADDR_REGION2_TX(vaddr)) {
@@ -244,6 +154,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         if (VADDR_REGION3_TL(vaddr) > asce_tl) {
             return PGM_REG_THIRD_TRANS;
         }
+        gaddr += VADDR_REGION3_TX(vaddr) * 8;
         break;
     case ASCE_TYPE_SEGMENT:
         if (VADDR_REGION1_TX(vaddr) || VADDR_REGION2_TX(vaddr) ||
@@ -253,11 +164,108 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         if (VADDR_SEGMENT_TL(vaddr) > asce_tl) {
             return PGM_SEGMENT_TRANS;
         }
+        gaddr += VADDR_SEGMENT_TX(vaddr) * 8;
+        break;
+    }
+
+    switch (asce & ASCE_TYPE_MASK) {
+    case ASCE_TYPE_REGION1:
+        if (!read_table_entry(env, gaddr, &entry)) {
+            return PGM_ADDRESSING;
+        }
+        if (entry & REGION_ENTRY_I) {
+            return PGM_REG_FIRST_TRANS;
+        }
+        if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION1) {
+            return PGM_TRANS_SPEC;
+        }
+        if (VADDR_REGION2_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
+            VADDR_REGION2_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
+            return PGM_REG_SEC_TRANS;
+        }
+        if (edat1 && (entry & REGION_ENTRY_P)) {
+            *flags &= ~PAGE_WRITE;
+        }
+        gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_REGION2_TX(vaddr) * 8;
+        /* fall through */
+    case ASCE_TYPE_REGION2:
+        if (!read_table_entry(env, gaddr, &entry)) {
+            return PGM_ADDRESSING;
+        }
+        if (entry & REGION_ENTRY_I) {
+            return PGM_REG_SEC_TRANS;
+        }
+        if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION2) {
+            return PGM_TRANS_SPEC;
+        }
+        if (VADDR_REGION3_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
+            VADDR_REGION3_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
+            return PGM_REG_THIRD_TRANS;
+        }
+        if (edat1 && (entry & REGION_ENTRY_P)) {
+            *flags &= ~PAGE_WRITE;
+        }
+        gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_REGION3_TX(vaddr) * 8;
+        /* fall through */
+    case ASCE_TYPE_REGION3:
+        if (!read_table_entry(env, gaddr, &entry)) {
+            return PGM_ADDRESSING;
+        }
+        if (entry & REGION_ENTRY_I) {
+            return PGM_REG_THIRD_TRANS;
+        }
+        if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) {
+            return PGM_TRANS_SPEC;
+        }
+        if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
+            VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
+            return PGM_SEGMENT_TRANS;
+        }
+        if (edat1 && (entry & REGION_ENTRY_P)) {
+            *flags &= ~PAGE_WRITE;
+        }
+        gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_SEGMENT_TX(vaddr) * 8;
+        /* fall through */
+    case ASCE_TYPE_SEGMENT:
+        if (!read_table_entry(env, gaddr, &entry)) {
+            return PGM_ADDRESSING;
+        }
+        if (entry & SEGMENT_ENTRY_I) {
+            return PGM_SEGMENT_TRANS;
+        }
+        if ((entry & SEGMENT_ENTRY_TT) != SEGMENT_ENTRY_TT_SEGMENT) {
+            return PGM_TRANS_SPEC;
+        }
+        if ((entry & SEGMENT_ENTRY_CS) && asce_p) {
+            return PGM_TRANS_SPEC;
+        }
+        if (entry & SEGMENT_ENTRY_P) {
+            *flags &= ~PAGE_WRITE;
+        }
+        if (edat1 && (entry & SEGMENT_ENTRY_FC)) {
+            *raddr = (entry & SEGMENT_ENTRY_SFAA) |
+                     (vaddr & ~SEGMENT_ENTRY_SFAA);
+            return 0;
+        }
+        gaddr = (entry & SEGMENT_ENTRY_ORIGIN) + VADDR_PAGE_TX(vaddr) * 8;
         break;
     }
 
-    return mmu_translate_region(env, vaddr, asc, asce, level, raddr, flags, rw,
-                                exc);
+    if (!read_table_entry(env, gaddr, &entry)) {
+        return PGM_ADDRESSING;
+    }
+    if (entry & PAGE_ENTRY_I) {
+        return PGM_PAGE_TRANS;
+    }
+    if (entry & PAGE_ENTRY_0) {
+        return PGM_TRANS_SPEC;
+    }
+    if (entry & PAGE_ENTRY_P) {
+        *flags &= ~PAGE_WRITE;
+    }
+
+    *raddr = entry & TARGET_PAGE_MASK;
+    return 0;
 }
 
 static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 08/31] s390x/mmu: Add EDAT2 translation support
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (6 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 07/31] s390x/mmu: Convert to non-recursive page table walk David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 09/31] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility David Hildenbrand
                   ` (24 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

This only adds basic support to the DAT translation, but no EDAT2 support
for TCG. E.g., the gdbstub under kvm uses this function, too, to
translate virtual addresses.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index dc33c63b1d..e27e21328b 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -120,6 +120,7 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
 {
     const bool edat1 = (env->cregs[0] & CR0_EDAT) &&
                        s390_has_feat(S390_FEAT_EDAT);
+    const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2);
     const int asce_tl = asce & ASCE_TABLE_LENGTH;
     const int asce_p = asce & ASCE_PRIVATE_SPACE;
     hwaddr gaddr = asce & ASCE_ORIGIN;
@@ -217,13 +218,21 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
         if ((entry & REGION_ENTRY_TT) != REGION_ENTRY_TT_REGION3) {
             return PGM_TRANS_SPEC;
         }
-        if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
-            VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
-            return PGM_SEGMENT_TRANS;
+        if (edat2 && (entry & REGION3_ENTRY_CR) && asce_p) {
+            return PGM_TRANS_SPEC;
         }
         if (edat1 && (entry & REGION_ENTRY_P)) {
             *flags &= ~PAGE_WRITE;
         }
+        if (edat2 && (entry & REGION3_ENTRY_FC)) {
+            *raddr = (entry & REGION3_ENTRY_RFAA) |
+                     (vaddr & ~REGION3_ENTRY_RFAA);
+            return 0;
+        }
+        if (VADDR_SEGMENT_TL(vaddr) < (entry & REGION_ENTRY_TF) >> 6 ||
+            VADDR_SEGMENT_TL(vaddr) > (entry & REGION_ENTRY_TL)) {
+            return PGM_SEGMENT_TRANS;
+        }
         gaddr = (entry & REGION_ENTRY_ORIGIN) + VADDR_SEGMENT_TX(vaddr) * 8;
         /* fall through */
     case ASCE_TYPE_SEGMENT:
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 09/31] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (7 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 08/31] s390x/mmu: Add EDAT2 translation support David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 10/31] s390x/mmu: Implement Instruction-Execution-Protection Facility David Hildenbrand
                   ` (23 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

We already implement ESOP-1. For ESOP-2, we only have to indicate all
protection exceptions properly. Due to EDAT-1, we already indicate DAT
exceptions properly. We don't trigger KCP/ALCP/IEP exceptions yet.

So all we have to do is set the TEID (TEC) to the right values
(bit 56, 60, 61) in case of LAP.

We don't have any side-effects (e.g., no guarded-storage facility),
therefore, bit 64 of the TEID (TEC) is always 0.

We always have to indicate whether it is a fetch or a store for all access
exceptions. This is only missing for LAP exceptions.

Acked-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index e27e21328b..06502bd25d 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -380,7 +380,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
             if (exc) {
-                trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
+                /* LAP sets bit 56 */
+                tec |= 0x80;
+                trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
             }
             return -EACCES;
         }
@@ -536,6 +538,9 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
 int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
                        target_ulong *addr, int *flags)
 {
+    /* Code accesses have an undefined ilc, let's use 2 bytes. */
+    uint64_t tec = (raddr & TARGET_PAGE_MASK) |
+                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
 
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -543,7 +548,9 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
         /* see comment in mmu_translate() how this works */
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
-            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, 0);
+            /* LAP sets bit 56 */
+            tec |= 0x80;
+            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, tec);
             return -EACCES;
         }
     }
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 10/31] s390x/mmu: Implement Instruction-Execution-Protection Facility
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (8 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 09/31] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 11/31] s390x/cpumodel: Prepare for changes of QEMU model David Hildenbrand
                   ` (22 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

IEP support in the mmu is fairly easy. Set the right permissions for TLB
entries and properly report an exception.

Make sure to handle EDAT-2 by setting bit 56/60/61 of the TEID (TEC) to
the right values.

Let's keep s390_cpu_get_phys_page_debug() working even if IEP is
active. Switch MMU_DATA_LOAD - this has no other effects any more as the
ASC to be used is now fully selected outside of mmu_translate().

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h        |  1 +
 target/s390x/helper.c     |  6 +++++-
 target/s390x/mmu_helper.c | 21 +++++++++++++++++++++
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 2db54884b8..b907741858 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -315,6 +315,7 @@ extern const VMStateDescription vmstate_s390_cpu;
 #define CR0_EDAT                0x0000000000800000ULL
 #define CR0_AFP                 0x0000000000040000ULL
 #define CR0_VECTOR              0x0000000000020000ULL
+#define CR0_IEP                 0x0000000000100000ULL
 #define CR0_EMERGENCY_SIGNAL_SC 0x0000000000004000ULL
 #define CR0_EXTERNAL_CALL_SC    0x0000000000002000ULL
 #define CR0_CKC_SC              0x0000000000000800ULL
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index 948c0398d4..bf503b56ee 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -63,7 +63,11 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
         asc = PSW_ASC_PRIMARY;
     }
 
-    if (mmu_translate(env, vaddr, MMU_INST_FETCH, asc, &raddr, &prot, false)) {
+    /*
+     * We want to read code even if IEP is active. Use MMU_DATA_LOAD instead
+     * of MMU_INST_FETCH.
+     */
+    if (mmu_translate(env, vaddr, MMU_DATA_LOAD, asc, &raddr, &prot, false)) {
         return -1;
     }
     return raddr;
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 06502bd25d..4a794dadcf 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -121,6 +121,8 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     const bool edat1 = (env->cregs[0] & CR0_EDAT) &&
                        s390_has_feat(S390_FEAT_EDAT);
     const bool edat2 = edat1 && s390_has_feat(S390_FEAT_EDAT_2);
+    const bool iep = (env->cregs[0] & CR0_IEP) &&
+                     s390_has_feat(S390_FEAT_INSTRUCTION_EXEC_PROT);
     const int asce_tl = asce & ASCE_TABLE_LENGTH;
     const int asce_p = asce & ASCE_PRIVATE_SPACE;
     hwaddr gaddr = asce & ASCE_ORIGIN;
@@ -225,6 +227,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
             *flags &= ~PAGE_WRITE;
         }
         if (edat2 && (entry & REGION3_ENTRY_FC)) {
+            if (iep && (entry & REGION3_ENTRY_IEP)) {
+                *flags &= ~PAGE_EXEC;
+            }
             *raddr = (entry & REGION3_ENTRY_RFAA) |
                      (vaddr & ~REGION3_ENTRY_RFAA);
             return 0;
@@ -252,6 +257,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
             *flags &= ~PAGE_WRITE;
         }
         if (edat1 && (entry & SEGMENT_ENTRY_FC)) {
+            if (iep && (entry & SEGMENT_ENTRY_IEP)) {
+                *flags &= ~PAGE_EXEC;
+            }
             *raddr = (entry & SEGMENT_ENTRY_SFAA) |
                      (vaddr & ~SEGMENT_ENTRY_SFAA);
             return 0;
@@ -272,6 +280,9 @@ static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
     if (entry & PAGE_ENTRY_P) {
         *flags &= ~PAGE_WRITE;
     }
+    if (iep && (entry & PAGE_ENTRY_IEP)) {
+        *flags &= ~PAGE_EXEC;
+    }
 
     *raddr = entry & TARGET_PAGE_MASK;
     return 0;
@@ -430,6 +441,16 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
         return -1;
     }
 
+    /* check for Instruction-Execution-Protection */
+    if (unlikely(rw == MMU_INST_FETCH && !(*flags & PAGE_EXEC))) {
+        if (exc) {
+            /* IEP sets bit 56 and 61 */
+            tec |= 0x84;
+            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
+        }
+        return -1;
+    }
+
 nodat:
     /* Convert real address -> absolute address */
     *raddr = mmu_real2abs(env, *raddr);
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 11/31] s390x/cpumodel: Prepare for changes of QEMU model
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (9 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 10/31] s390x/mmu: Implement Instruction-Execution-Protection Facility David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 12/31] s390x/cpumodel: Add new TCG features to QEMU cpu model David Hildenbrand
                   ` (21 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

Setup the 4.1 compatibility model so we can add new features to the
LATEST model.

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c  | 2 ++
 target/s390x/gen-features.c | 6 +++++-
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 18ad279a00..d3edeef0ad 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -650,7 +650,9 @@ DEFINE_CCW_MACHINE(4_2, "4.2", true);
 
 static void ccw_machine_4_1_instance_options(MachineState *machine)
 {
+    static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V4_1 };
     ccw_machine_4_2_instance_options(machine);
+    s390_set_qemu_cpu_model(0x2964, 13, 2, qemu_cpu_feat);
 }
 
 static void ccw_machine_4_1_class_options(MachineClass *mc)
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 49a650ac52..7e82f2f004 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -698,11 +698,14 @@ static uint16_t qemu_V4_0[] = {
     S390_FEAT_ZPCI,
 };
 
-static uint16_t qemu_LATEST[] = {
+static uint16_t qemu_V4_1[] = {
     S390_FEAT_STFLE_53,
     S390_FEAT_VECTOR,
 };
 
+static uint16_t qemu_LATEST[] = {
+};
+
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
     /* generates a dependency warning, leave it out for now */
@@ -824,6 +827,7 @@ static FeatGroupDefSpec QemuFeatDef[] = {
     QEMU_FEAT_INITIALIZER(V2_11),
     QEMU_FEAT_INITIALIZER(V3_1),
     QEMU_FEAT_INITIALIZER(V4_0),
+    QEMU_FEAT_INITIALIZER(V4_1),
     QEMU_FEAT_INITIALIZER(LATEST),
     QEMU_FEAT_INITIALIZER(MAX),
 };
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 12/31] s390x/cpumodel: Add new TCG features to QEMU cpu model
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (10 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 11/31] s390x/cpumodel: Prepare for changes of QEMU model David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 13/31] target/s390x: Add ilen to unwind data David Hildenbrand
                   ` (20 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

We now implement a bunch of new facilities we can properly indicate.

ESOP-1/ESOP-2 handling is discussed in the PoP Chafter 3-15
("Suppression on Protection"). The "Basic suppression-on-protection (SOP)
facility" is a core part of z/Architecture without a facility
indication. ESOP-2 is indicated by ESOP-1 + Side-effect facility
("ESOP-2"). Besides ESOP-2, the side-effect facility is only relevant for
the guarded-storage facility (we don't implement).

S390_ESOP:
- We indicate DAT exeptions by setting bit 61 of the TEID (TEC) to 1 and
  bit 60 to zero. We don't trigger ALCP exceptions yet. Also, we set
  bit 0-51 and bit 62/63 to the right values.
S390_ACCESS_EXCEPTION_FS_INDICATION:
- The TEID (TEC) properly indicates in bit 52/53 on any access if it was
  a fetch or a store
S390_SIDE_EFFECT_ACCESS_ESOP2:
- We have no side-effect accesses (esp., we don't implement the
  guarded-storage faciliy), we correctly set bit 64 of the TEID (TEC) to
  0 (no side-effect).
- ESOP2: We properly set bit 56, 60, 61 in the TEID (TEC) to indicate the
  type of protection. We don't trigger KCP/ALCP exceptions yet.
S390_INSTRUCTION_EXEC_PROT:
- The MMU properly detects and indicates the exception on instruction fetches
- Protected TLB entries will never get PAGE_EXEC set.

There is no need to fake the abscence of any of the facilities - without
the facilities, some bits of the TEID (TEC) are simply unpredictable.

As IEP was added with z14 and we currently implement a z13, add it to
the MAX model instead.

Acked-by: Cornelia Huck <cohuck@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/gen-features.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 7e82f2f004..6278845b12 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -704,12 +704,17 @@ static uint16_t qemu_V4_1[] = {
 };
 
 static uint16_t qemu_LATEST[] = {
+    S390_FEAT_ACCESS_EXCEPTION_FS_INDICATION,
+    S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
+    S390_FEAT_ESOP,
 };
 
 /* add all new definitions before this point */
 static uint16_t qemu_MAX[] = {
     /* generates a dependency warning, leave it out for now */
     S390_FEAT_MSA_EXT_5,
+    /* features introduced after the z13 */
+    S390_FEAT_INSTRUCTION_EXEC_PROT,
 };
 
 /****** END FEATURE DEFS ******/
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 13/31] target/s390x: Add ilen to unwind data
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (11 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 12/31] s390x/cpumodel: Add new TCG features to QEMU cpu model David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 14/31] target/s390x: Remove ilen parameter from tcg_s390_program_interrupt David Hildenbrand
                   ` (19 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Thomas Huth, David Hildenbrand, Cornelia Huck, Richard Henderson,
	qemu-s390x, Richard Henderson

From: Richard Henderson <rth@twiddle.net>

Use ILEN_UNWIND to signal that we have in fact that cpu_restore_state
will have been called by the time we arrive in do_program_interrupt.

Signed-off-by: Richard Henderson <rth@twiddle.net>
Message-Id: <20191001171614.8405-2-richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h       |  4 +++-
 target/s390x/interrupt.c |  5 ++++-
 target/s390x/translate.c | 20 +++++++++++++++++---
 3 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index b907741858..1a5b1397da 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -34,7 +34,7 @@
 /* The z/Architecture has a strong memory model with some store-after-load re-ordering */
 #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
 
-#define TARGET_INSN_START_EXTRA_WORDS 1
+#define TARGET_INSN_START_EXTRA_WORDS 2
 
 #define MMU_MODE0_SUFFIX _primary
 #define MMU_MODE1_SUFFIX _secondary
@@ -804,6 +804,8 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo, void *puc);
 void s390_crw_mchk(void);
 void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
                        uint32_t io_int_parm, uint32_t io_int_word);
+/* instruction length set by unwind info */
+#define ILEN_UNWIND                 0
 /* automatically detect the instruction length */
 #define ILEN_AUTO                   0xff
 #define RA_IGNORED                  0
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index a841f7187d..30a9fb8852 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -28,7 +28,10 @@ void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
 
     cs->exception_index = EXCP_PGM;
     env->int_pgm_code = code;
-    env->int_pgm_ilen = ilen;
+    /* If ILEN_UNWIND, int_pgm_ilen already has the correct value.  */
+    if (ilen != ILEN_UNWIND) {
+        env->int_pgm_ilen = ilen;
+    }
 }
 
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
diff --git a/target/s390x/translate.c b/target/s390x/translate.c
index a3e43ff9ec..151dfa91fb 100644
--- a/target/s390x/translate.c
+++ b/target/s390x/translate.c
@@ -6309,6 +6309,9 @@ static DisasJumpType translate_one(CPUS390XState *env, DisasContext *s)
     /* Search for the insn in the table.  */
     insn = extract_insn(env, s, &f);
 
+    /* Emit insn_start now that we know the ILEN.  */
+    tcg_gen_insn_start(s->base.pc_next, s->cc_op, s->ilen);
+
     /* Not found means unimplemented/illegal opcode.  */
     if (insn == NULL) {
         qemu_log_mask(LOG_UNIMP, "unimplemented opcode 0x%02x%02x\n",
@@ -6463,9 +6466,6 @@ static void s390x_tr_tb_start(DisasContextBase *db, CPUState *cs)
 
 static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
 {
-    DisasContext *dc = container_of(dcbase, DisasContext, base);
-
-    tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
 }
 
 static bool s390x_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
@@ -6473,6 +6473,14 @@ static bool s390x_tr_breakpoint_check(DisasContextBase *dcbase, CPUState *cs,
 {
     DisasContext *dc = container_of(dcbase, DisasContext, base);
 
+    /*
+     * Emit an insn_start to accompany the breakpoint exception.
+     * The ILEN value is a dummy, since this does not result in
+     * an s390x exception, but an internal qemu exception which
+     * brings us back to interact with the gdbstub.
+     */
+    tcg_gen_insn_start(dc->base.pc_next, dc->cc_op, 2);
+
     dc->base.is_jmp = DISAS_PC_STALE;
     dc->do_debug = true;
     /* The address covered by the breakpoint must be included in
@@ -6567,8 +6575,14 @@ void restore_state_to_opc(CPUS390XState *env, TranslationBlock *tb,
                           target_ulong *data)
 {
     int cc_op = data[1];
+
     env->psw.addr = data[0];
+
+    /* Update the CC opcode if it is not already up-to-date.  */
     if ((cc_op != CC_OP_DYNAMIC) && (cc_op != CC_OP_STATIC)) {
         env->cc_op = cc_op;
     }
+
+    /* Record ILEN.  */
+    env->int_pgm_ilen = data[2];
 }
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 14/31] target/s390x: Remove ilen parameter from tcg_s390_program_interrupt
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (12 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 13/31] target/s390x: Add ilen to unwind data David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 15/31] target/s390x: Remove ilen parameter from s390_program_interrupt David Hildenbrand
                   ` (18 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Since we begin the operation with an unwind, we have the proper
value of ilen immediately available.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-3-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 10 +++++-----
 target/s390x/interrupt.c   |  2 +-
 target/s390x/tcg-stub.c    |  4 ++--
 target/s390x/tcg_s390x.h   |  4 ++--
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 892f659d5a..eb55e5ef53 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -34,15 +34,15 @@
 #include "hw/boards.h"
 #endif
 
-void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
-                                              int ilen, uintptr_t ra)
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
+                                              uint32_t code, uintptr_t ra)
 {
     CPUState *cs = env_cpu(env);
 
     cpu_restore_state(cs, ra, true);
     qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
                   env->psw.addr);
-    trigger_pgm_exception(env, code, ilen);
+    trigger_pgm_exception(env, code, ILEN_UNWIND);
     cpu_loop_exit(cs);
 }
 
@@ -60,7 +60,7 @@ void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
     if (env->cregs[0] & CR0_AFP) {
         env->fpc = deposit32(env->fpc, 8, 8, dxc);
     }
-    tcg_s390_program_interrupt(env, PGM_DATA, ILEN_AUTO, ra);
+    tcg_s390_program_interrupt(env, PGM_DATA, ra);
 }
 
 void QEMU_NORETURN tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
@@ -75,7 +75,7 @@ void QEMU_NORETURN tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
 
     /* Always store the VXC into the FPC, without AFP it is undefined */
     env->fpc = deposit32(env->fpc, 8, 8, vxc);
-    tcg_s390_program_interrupt(env, PGM_VECTOR_PROCESSING, ILEN_AUTO, ra);
+    tcg_s390_program_interrupt(env, PGM_VECTOR_PROCESSING, ra);
 }
 
 void HELPER(data_exception)(CPUS390XState *env, uint32_t dxc)
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 30a9fb8852..b798e2ecbe 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -40,7 +40,7 @@ void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
     if (kvm_enabled()) {
         kvm_s390_program_interrupt(env_archcpu(env), code);
     } else if (tcg_enabled()) {
-        tcg_s390_program_interrupt(env, code, ilen, ra);
+        tcg_s390_program_interrupt(env, code, ra);
     } else {
         g_assert_not_reached();
     }
diff --git a/target/s390x/tcg-stub.c b/target/s390x/tcg-stub.c
index 32adb7276a..d22c898802 100644
--- a/target/s390x/tcg-stub.c
+++ b/target/s390x/tcg-stub.c
@@ -18,8 +18,8 @@
 void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque)
 {
 }
-void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
-                                              int ilen, uintptr_t ra)
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
+                                              uint32_t code, uintptr_t ra)
 {
     g_assert_not_reached();
 }
diff --git a/target/s390x/tcg_s390x.h b/target/s390x/tcg_s390x.h
index 2813f9d48e..2f54ccb027 100644
--- a/target/s390x/tcg_s390x.h
+++ b/target/s390x/tcg_s390x.h
@@ -14,8 +14,8 @@
 #define TCG_S390X_H
 
 void tcg_s390_tod_updated(CPUState *cs, run_on_cpu_data opaque);
-void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env, uint32_t code,
-                                              int ilen, uintptr_t ra);
+void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
+                                              uint32_t code, uintptr_t ra);
 void QEMU_NORETURN tcg_s390_data_exception(CPUS390XState *env, uint32_t dxc,
                                            uintptr_t ra);
 void QEMU_NORETURN tcg_s390_vector_exception(CPUS390XState *env, uint32_t vxc,
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 15/31] target/s390x: Remove ilen parameter from s390_program_interrupt
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (13 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 14/31] target/s390x: Remove ilen parameter from tcg_s390_program_interrupt David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 16/31] target/s390x: Use tcg_s390_program_interrupt in TCG helpers David Hildenbrand
                   ` (17 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

This is no longer used, and many of the existing uses -- particularly
within hw/s390x -- seem questionable.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-4-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 hw/s390x/s390-pci-inst.c     | 58 ++++++++++++++++++------------------
 target/s390x/cc_helper.c     |  2 +-
 target/s390x/cpu.h           |  3 +-
 target/s390x/crypto_helper.c |  4 +--
 target/s390x/diag.c          | 14 ++++-----
 target/s390x/excp_helper.c   |  2 +-
 target/s390x/fpu_helper.c    |  6 ++--
 target/s390x/int_helper.c    | 14 ++++-----
 target/s390x/interrupt.c     |  3 +-
 target/s390x/ioinst.c        | 40 ++++++++++++-------------
 target/s390x/mem_helper.c    | 43 +++++++++++++-------------
 target/s390x/misc_helper.c   | 27 ++++++-----------
 12 files changed, 102 insertions(+), 114 deletions(-)

diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c
index 4b3bd4a804..92c7e45df5 100644
--- a/hw/s390x/s390-pci-inst.c
+++ b/hw/s390x/s390-pci-inst.c
@@ -157,7 +157,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
     int i;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 4, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
@@ -168,7 +168,7 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
     reqh = (ClpReqHdr *)buffer;
     req_len = lduw_p(&reqh->len);
     if (req_len < 16 || req_len > 8184 || (req_len % 8 != 0)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return 0;
     }
 
@@ -180,11 +180,11 @@ int clp_service_call(S390CPU *cpu, uint8_t r2, uintptr_t ra)
     resh = (ClpRspHdr *)(buffer + req_len);
     res_len = lduw_p(&resh->len);
     if (res_len < 8 || res_len > 8176 || (res_len % 8 != 0)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return 0;
     }
     if ((req_len + res_len) > 8192) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return 0;
     }
 
@@ -390,12 +390,12 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
     uint8_t pcias;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 4, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
     if (r2 & 0x1) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
@@ -429,25 +429,25 @@ int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
     switch (pcias) {
     case ZPCI_IO_BAR_MIN...ZPCI_IO_BAR_MAX:
         if (!len || (len > (8 - (offset & 0x7)))) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         result = zpci_read_bar(pbdev, pcias, offset, &data, len);
         if (result != MEMTX_OK) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         break;
     case ZPCI_CONFIG_BAR:
         if (!len || (len > (4 - (offset & 0x3))) || len == 3) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         data =  pci_host_config_read_common(
                    pbdev->pdev, offset, pci_config_size(pbdev->pdev), len);
 
         if (zpci_endian_swap(&data, len)) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         break;
@@ -489,12 +489,12 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
     uint8_t pcias;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 4, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
     if (r2 & 0x1) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
@@ -536,13 +536,13 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
          * A length of 0 is invalid and length should not cross a double word
          */
         if (!len || (len > (8 - (offset & 0x7)))) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
 
         result = zpci_write_bar(pbdev, pcias, offset, data, len);
         if (result != MEMTX_OK) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         break;
@@ -550,7 +550,7 @@ int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
         /* ZPCI uses the pseudo BAR number 15 as configuration space */
         /* possible access lengths are 1,2,4 and must not cross a word */
         if (!len || (len > (4 - (offset & 0x3))) || len == 3) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
         /* len = 1,2,4 so we do not need to test */
@@ -622,12 +622,12 @@ int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2, uintptr_t ra)
     hwaddr start, end;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 4, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
     if (r2 & 0x1) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
@@ -709,7 +709,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     uint8_t buffer[128];
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 6, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
@@ -772,7 +772,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
 
     if (!memory_region_access_valid(mr, offset, len, true,
                                     MEMTXATTRS_UNSPECIFIED)) {
-        s390_program_interrupt(env, PGM_OPERAND, 6, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return 0;
     }
 
@@ -786,7 +786,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
                                               ldq_p(buffer + i * 8),
                                               MO_64, MEMTXATTRS_UNSPECIFIED);
         if (result != MEMTX_OK) {
-            s390_program_interrupt(env, PGM_OPERAND, 6, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
             return 0;
         }
     }
@@ -797,7 +797,7 @@ int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
     return 0;
 
 specification_error:
-    s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+    s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     return 0;
 }
 
@@ -871,14 +871,14 @@ static int reg_ioat(CPUS390XState *env, S390PCIIOMMU *iommu, ZpciFib fib,
     pba &= ~0xfff;
     pal |= 0xfff;
     if (pba > pal || pba < ZPCI_SDMA_ADDR || pal > ZPCI_EDMA_ADDR) {
-        s390_program_interrupt(env, PGM_OPERAND, 6, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return -EINVAL;
     }
 
     /* currently we only support designation type 1 with translation */
     if (!(dt == ZPCI_IOTA_RTTO && t)) {
         error_report("unsupported ioat dt %d t %d", dt, t);
-        s390_program_interrupt(env, PGM_OPERAND, 6, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return -EINVAL;
     }
 
@@ -1003,7 +1003,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     uint64_t cc = ZPCI_PCI_LS_OK;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 6, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
@@ -1012,7 +1012,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     fh = env->regs[r1] >> 32;
 
     if (fiba & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
@@ -1040,7 +1040,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     }
 
     if (fib.fmt != 0) {
-        s390_program_interrupt(env, PGM_OPERAND, 6, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return 0;
     }
 
@@ -1151,7 +1151,7 @@ int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
         break;
     }
     default:
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 6, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
         cc = ZPCI_PCI_LS_ERR;
     }
 
@@ -1171,7 +1171,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     uint64_t cc = ZPCI_PCI_LS_OK;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 6, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return 0;
     }
 
@@ -1185,7 +1185,7 @@ int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar,
     }
 
     if (fiba & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index cf68792733..3cb00bcb09 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -588,7 +588,7 @@ void HELPER(sacf)(CPUS390XState *env, uint64_t a1)
         break;
     default:
         HELPER_LOG("unknown sacf mode: %" PRIx64 "\n", a1);
-        s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
         break;
     }
 }
diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 1a5b1397da..18e7091763 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -809,8 +809,7 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
 /* automatically detect the instruction length */
 #define ILEN_AUTO                   0xff
 #define RA_IGNORED                  0
-void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
-                            uintptr_t ra);
+void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
 void s390_sclp_extint(uint32_t parm);
 
diff --git a/target/s390x/crypto_helper.c b/target/s390x/crypto_helper.c
index 5c79790187..1f83987e9d 100644
--- a/target/s390x/crypto_helper.c
+++ b/target/s390x/crypto_helper.c
@@ -34,7 +34,7 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
     case S390_FEAT_TYPE_PCKMO:
     case S390_FEAT_TYPE_PCC:
         if (mod) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
             return 0;
         }
         break;
@@ -42,7 +42,7 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
 
     s390_get_feat_block(type, subfunc);
     if (!test_be_bit(fc, subfunc)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return 0;
     }
 
diff --git a/target/s390x/diag.c b/target/s390x/diag.c
index 65eabf0461..53c2f81f2a 100644
--- a/target/s390x/diag.c
+++ b/target/s390x/diag.c
@@ -61,12 +61,12 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
     IplParameterBlock *iplb;
 
     if (env->psw.mask & PSW_MASK_PSTATE) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
         return;
     }
 
     if ((subcode & ~0x0ffffULL) || (subcode > 6)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
 
@@ -82,13 +82,13 @@ void handle_diag_308(CPUS390XState *env, uint64_t r1, uint64_t r3, uintptr_t ra)
         break;
     case 5:
         if ((r1 & 1) || (addr & 0x0fffULL)) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
                                         sizeof(IplParameterBlock), false,
                                         MEMTXATTRS_UNSPECIFIED)) {
-            s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
+            s390_program_interrupt(env, PGM_ADDRESSING, ra);
             return;
         }
         iplb = g_new0(IplParameterBlock, 1);
@@ -112,13 +112,13 @@ out:
         return;
     case 6:
         if ((r1 & 1) || (addr & 0x0fffULL)) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
             return;
         }
         if (!address_space_access_valid(&address_space_memory, addr,
                                         sizeof(IplParameterBlock), true,
                                         MEMTXATTRS_UNSPECIFIED)) {
-            s390_program_interrupt(env, PGM_ADDRESSING, ILEN_AUTO, ra);
+            s390_program_interrupt(env, PGM_ADDRESSING, ra);
             return;
         }
         iplb = s390_ipl_get_iplb();
@@ -130,7 +130,7 @@ out:
         }
         return;
     default:
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         break;
     }
 }
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index eb55e5ef53..089623a248 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -614,7 +614,7 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
 
-    s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, retaddr);
+    s390_program_interrupt(env, PGM_SPECIFICATION, retaddr);
 }
 
 #endif /* CONFIG_USER_ONLY */
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 5faf973c6c..7228eb96e2 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -825,7 +825,7 @@ void HELPER(sfpc)(CPUS390XState *env, uint64_t fpc)
 {
     if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u ||
         (!s390_has_feat(S390_FEAT_FLOATING_POINT_EXT) && fpc & 0x4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     /* Install everything in the main FPC.  */
@@ -843,7 +843,7 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
 
     if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u ||
         (!s390_has_feat(S390_FEAT_FLOATING_POINT_EXT) && fpc & 0x4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     /*
@@ -880,7 +880,7 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
 void HELPER(srnm)(CPUS390XState *env, uint64_t rnd)
 {
     if (rnd > 0x7 || fpc_to_rnd[rnd & 0x7] == -1) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     env->fpc = deposit32(env->fpc, 0, 3, rnd);
diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
index d13cc49be6..1d29a1fc1f 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/int_helper.c
@@ -39,7 +39,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)
     int64_t q;
 
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     ret = q = a / b;
@@ -47,7 +47,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)
 
     /* Catch non-representable quotient.  */
     if (ret != q) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     return ret;
@@ -60,7 +60,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)
     uint64_t q;
 
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     ret = q = a / b;
@@ -68,7 +68,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)
 
     /* Catch non-representable quotient.  */
     if (ret != q) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     return ret;
@@ -79,7 +79,7 @@ int64_t HELPER(divs64)(CPUS390XState *env, int64_t a, int64_t b)
 {
     /* Catch divide by zero, and non-representable quotient (MIN / -1).  */
     if (b == 0 || (b == -1 && a == (1ll << 63))) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
     env->retxl = a % b;
     return a / b;
@@ -92,7 +92,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
     uint64_t ret;
     /* Signal divide by zero.  */
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
     if (ah == 0) {
         /* 64 -> 64/64 case */
@@ -106,7 +106,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
         env->retxl = a % b;
         ret = q;
         if (ret != q) {
-            s390_program_interrupt(env, PGM_FIXPT_DIVIDE, ILEN_AUTO, GETPC());
+            s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
         }
 #else
         /* 32-bit hosts would need special wrapper functionality - just abort if
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index b798e2ecbe..2b71e03914 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -34,8 +34,7 @@ void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
     }
 }
 
-void s390_program_interrupt(CPUS390XState *env, uint32_t code, int ilen,
-                            uintptr_t ra)
+void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra)
 {
     if (kvm_enabled()) {
         kvm_s390_program_interrupt(env_archcpu(env), code);
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 83c164a168..c437a1d8c6 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -44,7 +44,7 @@ void ioinst_handle_xsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("xsch", cssid, ssid, schid);
@@ -62,7 +62,7 @@ void ioinst_handle_csch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("csch", cssid, ssid, schid);
@@ -80,7 +80,7 @@ void ioinst_handle_hsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("hsch", cssid, ssid, schid);
@@ -116,7 +116,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &schib, sizeof(schib))) {
@@ -125,7 +125,7 @@ void ioinst_handle_msch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     }
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
         !ioinst_schib_valid(&schib)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("msch", cssid, ssid, schid);
@@ -173,7 +173,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
     if (s390_cpu_virt_mem_read(cpu, addr, ar, &orig_orb, sizeof(orb))) {
@@ -183,7 +183,7 @@ void ioinst_handle_ssch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     copy_orb_from_guest(&orb, &orig_orb);
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid) ||
         !ioinst_orb_valid(&orb)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("ssch", cssid, ssid, schid);
@@ -205,7 +205,7 @@ void ioinst_handle_stcrw(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
 
@@ -236,7 +236,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
 
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
 
@@ -247,7 +247,7 @@ void ioinst_handle_stsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb,
          * access execption if it is not) first.
          */
         if (!s390_cpu_virt_mem_check_write(cpu, addr, ar, sizeof(schib))) {
-            s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+            s390_program_interrupt(env, PGM_OPERAND, ra);
         } else {
             s390_cpu_virt_mem_handle_exc(cpu, ra);
         }
@@ -299,13 +299,13 @@ int ioinst_handle_tsch(S390CPU *cpu, uint64_t reg1, uint32_t ipb, uintptr_t ra)
     uint8_t ar;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return -EIO;
     }
     trace_ioinst_sch_id("tsch", cssid, ssid, schid);
     addr = decode_basedisp_s(env, ipb, &ar);
     if (addr & 3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return -EIO;
     }
 
@@ -613,7 +613,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
     addr = env->regs[reg];
     /* Page boundary? */
     if (addr & 0xfff) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         return;
     }
     /*
@@ -629,7 +629,7 @@ void ioinst_handle_chsc(S390CPU *cpu, uint32_t ipb, uintptr_t ra)
     len = be16_to_cpu(req->len);
     /* Length field valid? */
     if ((len < 16) || (len > 4088) || (len & 7)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
     memset((char *)req + len, 0, TARGET_PAGE_SIZE - len);
@@ -678,7 +678,7 @@ void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
     trace_ioinst("schm");
 
     if (SCHM_REG1_RES(reg1)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
 
@@ -687,7 +687,7 @@ void ioinst_handle_schm(S390CPU *cpu, uint64_t reg1, uint64_t reg2,
     dct = SCHM_REG1_DCT(reg1);
 
     if (update && (reg2 & 0x000000000000001f)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
 
@@ -700,7 +700,7 @@ void ioinst_handle_rsch(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
     SubchDev *sch;
 
     if (ioinst_disassemble_sch_ident(reg1, &m, &cssid, &ssid, &schid)) {
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
         return;
     }
     trace_ioinst_sch_id("rsch", cssid, ssid, schid);
@@ -724,7 +724,7 @@ void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
     CPUS390XState *env = &cpu->env;
 
     if (RCHP_REG1_RES(reg1)) {
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
 
@@ -747,7 +747,7 @@ void ioinst_handle_rchp(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
         break;
     default:
         /* Invalid channel subsystem. */
-        s390_program_interrupt(env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(env, PGM_OPERAND, ra);
         return;
     }
     setcc(cpu, cc);
@@ -758,6 +758,6 @@ void ioinst_handle_sal(S390CPU *cpu, uint64_t reg1, uintptr_t ra)
 {
     /* We do not provide address limit checking, so let's suppress it. */
     if (SAL_REG1_INVALID(reg1) || reg1 & 0x000000000000ffff) {
-        s390_program_interrupt(&cpu->env, PGM_OPERAND, 4, ra);
+        s390_program_interrupt(&cpu->env, PGM_OPERAND, ra);
     }
 }
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 08c5cc6a99..77d2eb96d4 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -71,7 +71,7 @@ static inline void check_alignment(CPUS390XState *env, uint64_t v,
                                    int wordsize, uintptr_t ra)
 {
     if (v % wordsize) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 }
 
@@ -730,7 +730,7 @@ void HELPER(srst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-55 must contain all 0.  */
     if (env->regs[0] & 0xffffff00u) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     str = get_address(env, r2);
@@ -767,7 +767,7 @@ void HELPER(srstu)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-47 of R0 must be zero.  */
     if (env->regs[0] & 0xffff0000u) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     str = get_address(env, r2);
@@ -846,7 +846,7 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
     S390Access srca, desta;
 
     if ((f && s) || extract64(r0, 12, 4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     r1 = wrap_address(env, r1 & TARGET_PAGE_MASK);
@@ -879,7 +879,7 @@ uint32_t HELPER(mvst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     int i;
 
     if (env->regs[0] & 0xffffff00ull) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     /*
@@ -911,8 +911,7 @@ void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     int i;
 
     if (a2 & 0x3) {
-        /* we either came here by lam or lamy, which have different lengths */
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -932,7 +931,7 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     int i;
 
     if (a2 & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1888,7 +1887,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
     return cc;
 
  spec_exception:
-    s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+    s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     g_assert_not_reached();
 }
 
@@ -1912,7 +1911,7 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (src & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1945,7 +1944,7 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (src & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1976,7 +1975,7 @@ void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (dest & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1996,7 +1995,7 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (dest & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -2168,7 +2167,7 @@ uint32_t HELPER(mvcs)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 
     if (!(env->psw.mask & PSW_MASK_DAT) || !(env->cregs[0] & CR0_SECONDARY) ||
         psw_as == AS_HOME || psw_as == AS_ACCREG) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
 
     l = wrap_length32(env, l);
@@ -2199,7 +2198,7 @@ uint32_t HELPER(mvcp)(CPUS390XState *env, uint64_t l, uint64_t a1, uint64_t a2)
 
     if (!(env->psw.mask & PSW_MASK_DAT) || !(env->cregs[0] & CR0_SECONDARY) ||
         psw_as == AS_HOME || psw_as == AS_ACCREG) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, ILEN_AUTO, ra);
+        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
 
     l = wrap_length32(env, l);
@@ -2226,7 +2225,7 @@ void HELPER(idte)(CPUS390XState *env, uint64_t r1, uint64_t r2, uint32_t m4)
     uint16_t entries, i, index = 0;
 
     if (r2 & 0xff000) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     if (!(r2 & 0x800)) {
@@ -2370,7 +2369,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, 2, GETPC());
+        s390_program_interrupt(env, PGM_SPECIAL_OP, GETPC());
     }
 
     old_exc = cs->exception_index;
@@ -2539,7 +2538,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
                __func__, dest, src, len);
 
     if (!(env->psw.mask & PSW_MASK_DAT)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
 
     /* OAC (operand access control) for the first operand -> dest */
@@ -2570,14 +2569,14 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 
     if (dest_a && dest_as == AS_HOME && (env->psw.mask & PSW_MASK_PSTATE)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
     if (!(env->cregs[0] & CR0_SECONDARY) &&
         (dest_as == AS_SECONDARY || src_as == AS_SECONDARY)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, 6, ra);
+        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
     if (!psw_key_valid(env, dest_key) || !psw_key_valid(env, src_key)) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, 6, ra);
+        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
     }
 
     len = wrap_length32(env, len);
@@ -2591,7 +2590,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
         (env->psw.mask & PSW_MASK_PSTATE)) {
         qemu_log_mask(LOG_UNIMP, "%s: AR-mode and PSTATE support missing\n",
                       __func__);
-        s390_program_interrupt(env, PGM_ADDRESSING, 6, ra);
+        s390_program_interrupt(env, PGM_ADDRESSING, ra);
     }
 
     /* FIXME: Access using correct keys and AR-mode */
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 7530dcb8f3..9fbb37cfb9 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -106,7 +106,7 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
     int r = sclp_service_call(env, r1, r2);
     qemu_mutex_unlock_iothread();
     if (r < 0) {
-        s390_program_interrupt(env, -r, 4, GETPC());
+        s390_program_interrupt(env, -r, GETPC());
     }
     return r;
 }
@@ -143,7 +143,7 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
     }
 
     if (r) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ILEN_AUTO, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 }
 
@@ -222,7 +222,7 @@ void HELPER(sckpf)(CPUS390XState *env, uint64_t r0)
     uint32_t val = r0;
 
     if (val & 0xffff0000) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 2, GETPC());
+        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
     env->todpr = val;
 }
@@ -266,7 +266,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
     }
 
     if ((r0 & STSI_R0_RESERVED_MASK) || (r1 & STSI_R1_RESERVED_MASK)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     if ((r0 & STSI_R0_FC_MASK) == STSI_R0_FC_CURRENT) {
@@ -276,7 +276,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
     }
 
     if (a0 & ~TARGET_PAGE_MASK) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     /* count the cpus and split them into configured and reserved ones */
@@ -509,7 +509,7 @@ uint32_t HELPER(tpi)(CPUS390XState *env, uint64_t addr)
     LowCore *lowcore;
 
     if (addr & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     qemu_mutex_lock_iothread();
@@ -573,17 +573,8 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
 #ifndef CONFIG_USER_ONLY
 void HELPER(per_check_exception)(CPUS390XState *env)
 {
-    uint32_t ilen;
-
     if (env->per_perc_atmid) {
-        /*
-         * FIXME: ILEN_AUTO is most probably the right thing to use. ilen
-         * always has to match the instruction referenced in the PSW. E.g.
-         * if a PER interrupt is triggered via EXECUTE, we have to use ilen
-         * of EXECUTE, while per_address contains the target of EXECUTE.
-         */
-        ilen = get_ilen(cpu_ldub_code(env, env->per_address));
-        s390_program_interrupt(env, PGM_PER, ilen, GETPC());
+        s390_program_interrupt(env, PGM_PER, GETPC());
     }
 }
 
@@ -673,7 +664,7 @@ uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
     int i;
 
     if (addr & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, 4, ra);
+        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     prepare_stfl();
@@ -746,7 +737,7 @@ void HELPER(sic)(CPUS390XState *env, uint64_t r1, uint64_t r3)
     qemu_mutex_unlock_iothread();
     /* css_do_sic() may actually return a PGM_xxx value to inject */
     if (r) {
-        s390_program_interrupt(env, -r, 4, GETPC());
+        s390_program_interrupt(env, -r, GETPC());
     }
 }
 
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 16/31] target/s390x: Use tcg_s390_program_interrupt in TCG helpers
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (14 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 15/31] target/s390x: Remove ilen parameter from s390_program_interrupt David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 17/31] target/s390x: Push trigger_pgm_exception lower in s390_cpu_tlb_fill David Hildenbrand
                   ` (16 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Replace all uses of s390_program_interrupt within files
that are marked CONFIG_TCG.  These are necessarily tcg-only.

This lets each of these users benefit from the QEMU_NORETURN
attribute on tcg_s390_program_interrupt.

Acked-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-5-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cc_helper.c     |  4 ++--
 target/s390x/crypto_helper.c |  7 +++----
 target/s390x/excp_helper.c   |  2 +-
 target/s390x/fpu_helper.c    |  6 +++---
 target/s390x/int_helper.c    | 15 +++++++-------
 target/s390x/mem_helper.c    | 40 ++++++++++++++++++------------------
 target/s390x/misc_helper.c   | 18 ++++++++--------
 7 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/target/s390x/cc_helper.c b/target/s390x/cc_helper.c
index 3cb00bcb09..44731e4a85 100644
--- a/target/s390x/cc_helper.c
+++ b/target/s390x/cc_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
+#include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "exec/helper-proto.h"
 #include "qemu/host-utils.h"
@@ -588,8 +589,7 @@ void HELPER(sacf)(CPUS390XState *env, uint64_t a1)
         break;
     default:
         HELPER_LOG("unknown sacf mode: %" PRIx64 "\n", a1);
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
-        break;
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 }
 #endif
diff --git a/target/s390x/crypto_helper.c b/target/s390x/crypto_helper.c
index 1f83987e9d..ff3fbc3950 100644
--- a/target/s390x/crypto_helper.c
+++ b/target/s390x/crypto_helper.c
@@ -13,6 +13,7 @@
 #include "qemu/osdep.h"
 #include "qemu/main-loop.h"
 #include "internal.h"
+#include "tcg_s390x.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
@@ -34,16 +35,14 @@ uint32_t HELPER(msa)(CPUS390XState *env, uint32_t r1, uint32_t r2, uint32_t r3,
     case S390_FEAT_TYPE_PCKMO:
     case S390_FEAT_TYPE_PCC:
         if (mod) {
-            s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-            return 0;
+            tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
         }
         break;
     }
 
     s390_get_feat_block(type, subfunc);
     if (!test_be_bit(fc, subfunc)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-        return 0;
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     switch (fc) {
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 089623a248..dbff772d34 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -614,7 +614,7 @@ void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
 
-    s390_program_interrupt(env, PGM_SPECIFICATION, retaddr);
+    tcg_s390_program_interrupt(env, PGM_SPECIFICATION, retaddr);
 }
 
 #endif /* CONFIG_USER_ONLY */
diff --git a/target/s390x/fpu_helper.c b/target/s390x/fpu_helper.c
index 7228eb96e2..8bb9f54fd0 100644
--- a/target/s390x/fpu_helper.c
+++ b/target/s390x/fpu_helper.c
@@ -825,7 +825,7 @@ void HELPER(sfpc)(CPUS390XState *env, uint64_t fpc)
 {
     if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u ||
         (!s390_has_feat(S390_FEAT_FLOATING_POINT_EXT) && fpc & 0x4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     /* Install everything in the main FPC.  */
@@ -843,7 +843,7 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
 
     if (fpc_to_rnd[fpc & 0x7] == -1 || fpc & 0x03030088u ||
         (!s390_has_feat(S390_FEAT_FLOATING_POINT_EXT) && fpc & 0x4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     /*
@@ -880,7 +880,7 @@ void HELPER(sfas)(CPUS390XState *env, uint64_t fpc)
 void HELPER(srnm)(CPUS390XState *env, uint64_t rnd)
 {
     if (rnd > 0x7 || fpc_to_rnd[rnd & 0x7] == -1) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     env->fpc = deposit32(env->fpc, 0, 3, rnd);
diff --git a/target/s390x/int_helper.c b/target/s390x/int_helper.c
index 1d29a1fc1f..658507dd6d 100644
--- a/target/s390x/int_helper.c
+++ b/target/s390x/int_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
+#include "tcg_s390x.h"
 #include "exec/exec-all.h"
 #include "qemu/host-utils.h"
 #include "exec/helper-proto.h"
@@ -39,7 +40,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)
     int64_t q;
 
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     ret = q = a / b;
@@ -47,7 +48,7 @@ int64_t HELPER(divs32)(CPUS390XState *env, int64_t a, int64_t b64)
 
     /* Catch non-representable quotient.  */
     if (ret != q) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     return ret;
@@ -60,7 +61,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)
     uint64_t q;
 
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     ret = q = a / b;
@@ -68,7 +69,7 @@ uint64_t HELPER(divu32)(CPUS390XState *env, uint64_t a, uint64_t b64)
 
     /* Catch non-representable quotient.  */
     if (ret != q) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
 
     return ret;
@@ -79,7 +80,7 @@ int64_t HELPER(divs64)(CPUS390XState *env, int64_t a, int64_t b)
 {
     /* Catch divide by zero, and non-representable quotient (MIN / -1).  */
     if (b == 0 || (b == -1 && a == (1ll << 63))) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
     env->retxl = a % b;
     return a / b;
@@ -92,7 +93,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
     uint64_t ret;
     /* Signal divide by zero.  */
     if (b == 0) {
-        s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+        tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
     }
     if (ah == 0) {
         /* 64 -> 64/64 case */
@@ -106,7 +107,7 @@ uint64_t HELPER(divu64)(CPUS390XState *env, uint64_t ah, uint64_t al,
         env->retxl = a % b;
         ret = q;
         if (ret != q) {
-            s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
+            tcg_s390_program_interrupt(env, PGM_FIXPT_DIVIDE, GETPC());
         }
 #else
         /* 32-bit hosts would need special wrapper functionality - just abort if
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 77d2eb96d4..7d2a652823 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -21,6 +21,7 @@
 #include "qemu/osdep.h"
 #include "cpu.h"
 #include "internal.h"
+#include "tcg_s390x.h"
 #include "exec/helper-proto.h"
 #include "exec/exec-all.h"
 #include "exec/cpu_ldst.h"
@@ -71,7 +72,7 @@ static inline void check_alignment(CPUS390XState *env, uint64_t v,
                                    int wordsize, uintptr_t ra)
 {
     if (v % wordsize) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 }
 
@@ -730,7 +731,7 @@ void HELPER(srst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-55 must contain all 0.  */
     if (env->regs[0] & 0xffffff00u) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     str = get_address(env, r2);
@@ -767,7 +768,7 @@ void HELPER(srstu)(CPUS390XState *env, uint32_t r1, uint32_t r2)
 
     /* Bits 32-47 of R0 must be zero.  */
     if (env->regs[0] & 0xffff0000u) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     str = get_address(env, r2);
@@ -846,7 +847,7 @@ uint32_t HELPER(mvpg)(CPUS390XState *env, uint64_t r0, uint64_t r1, uint64_t r2)
     S390Access srca, desta;
 
     if ((f && s) || extract64(r0, 12, 4)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 
     r1 = wrap_address(env, r1 & TARGET_PAGE_MASK);
@@ -879,7 +880,7 @@ uint32_t HELPER(mvst)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     int i;
 
     if (env->regs[0] & 0xffffff00ull) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     /*
@@ -911,7 +912,7 @@ void HELPER(lam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     int i;
 
     if (a2 & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -931,7 +932,7 @@ void HELPER(stam)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     int i;
 
     if (a2 & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1887,8 +1888,7 @@ static uint32_t do_csst(CPUS390XState *env, uint32_t r3, uint64_t a1,
     return cc;
 
  spec_exception:
-    s390_program_interrupt(env, PGM_SPECIFICATION, ra);
-    g_assert_not_reached();
+    tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
 }
 
 uint32_t HELPER(csst)(CPUS390XState *env, uint32_t r3, uint64_t a1, uint64_t a2)
@@ -1911,7 +1911,7 @@ void HELPER(lctlg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (src & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1944,7 +1944,7 @@ void HELPER(lctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (src & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1975,7 +1975,7 @@ void HELPER(stctg)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (dest & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -1995,7 +1995,7 @@ void HELPER(stctl)(CPUS390XState *env, uint32_t r1, uint64_t a2, uint32_t r3)
     uint32_t i;
 
     if (dest & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     for (i = r1;; i = (i + 1) % 16) {
@@ -2225,7 +2225,7 @@ void HELPER(idte)(CPUS390XState *env, uint64_t r1, uint64_t r2, uint32_t m4)
     uint16_t entries, i, index = 0;
 
     if (r2 & 0xff000) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     if (!(r2 & 0x800)) {
@@ -2369,7 +2369,7 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, GETPC());
     }
 
     old_exc = cs->exception_index;
@@ -2538,7 +2538,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
                __func__, dest, src, len);
 
     if (!(env->psw.mask & PSW_MASK_DAT)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
 
     /* OAC (operand access control) for the first operand -> dest */
@@ -2569,14 +2569,14 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
     }
 
     if (dest_a && dest_as == AS_HOME && (env->psw.mask & PSW_MASK_PSTATE)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
     if (!(env->cregs[0] & CR0_SECONDARY) &&
         (dest_as == AS_SECONDARY || src_as == AS_SECONDARY)) {
-        s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, ra);
     }
     if (!psw_key_valid(env, dest_key) || !psw_key_valid(env, src_key)) {
-        s390_program_interrupt(env, PGM_PRIVILEGED, ra);
+        tcg_s390_program_interrupt(env, PGM_PRIVILEGED, ra);
     }
 
     len = wrap_length32(env, len);
@@ -2590,7 +2590,7 @@ uint32_t HELPER(mvcos)(CPUS390XState *env, uint64_t dest, uint64_t src,
         (env->psw.mask & PSW_MASK_PSTATE)) {
         qemu_log_mask(LOG_UNIMP, "%s: AR-mode and PSTATE support missing\n",
                       __func__);
-        s390_program_interrupt(env, PGM_ADDRESSING, ra);
+        tcg_s390_program_interrupt(env, PGM_ADDRESSING, ra);
     }
 
     /* FIXME: Access using correct keys and AR-mode */
diff --git a/target/s390x/misc_helper.c b/target/s390x/misc_helper.c
index 9fbb37cfb9..bfb457fb63 100644
--- a/target/s390x/misc_helper.c
+++ b/target/s390x/misc_helper.c
@@ -106,7 +106,7 @@ uint32_t HELPER(servc)(CPUS390XState *env, uint64_t r1, uint64_t r2)
     int r = sclp_service_call(env, r1, r2);
     qemu_mutex_unlock_iothread();
     if (r < 0) {
-        s390_program_interrupt(env, -r, GETPC());
+        tcg_s390_program_interrupt(env, -r, GETPC());
     }
     return r;
 }
@@ -143,7 +143,7 @@ void HELPER(diag)(CPUS390XState *env, uint32_t r1, uint32_t r3, uint32_t num)
     }
 
     if (r) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
 }
 
@@ -222,7 +222,7 @@ void HELPER(sckpf)(CPUS390XState *env, uint64_t r0)
     uint32_t val = r0;
 
     if (val & 0xffff0000) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, GETPC());
     }
     env->todpr = val;
 }
@@ -266,7 +266,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
     }
 
     if ((r0 & STSI_R0_RESERVED_MASK) || (r1 & STSI_R1_RESERVED_MASK)) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     if ((r0 & STSI_R0_FC_MASK) == STSI_R0_FC_CURRENT) {
@@ -276,7 +276,7 @@ uint32_t HELPER(stsi)(CPUS390XState *env, uint64_t a0, uint64_t r0, uint64_t r1)
     }
 
     if (a0 & ~TARGET_PAGE_MASK) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     /* count the cpus and split them into configured and reserved ones */
@@ -509,7 +509,7 @@ uint32_t HELPER(tpi)(CPUS390XState *env, uint64_t addr)
     LowCore *lowcore;
 
     if (addr & 0x3) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     qemu_mutex_lock_iothread();
@@ -574,7 +574,7 @@ void HELPER(chsc)(CPUS390XState *env, uint64_t inst)
 void HELPER(per_check_exception)(CPUS390XState *env)
 {
     if (env->per_perc_atmid) {
-        s390_program_interrupt(env, PGM_PER, GETPC());
+        tcg_s390_program_interrupt(env, PGM_PER, GETPC());
     }
 }
 
@@ -664,7 +664,7 @@ uint32_t HELPER(stfle)(CPUS390XState *env, uint64_t addr)
     int i;
 
     if (addr & 0x7) {
-        s390_program_interrupt(env, PGM_SPECIFICATION, ra);
+        tcg_s390_program_interrupt(env, PGM_SPECIFICATION, ra);
     }
 
     prepare_stfl();
@@ -737,7 +737,7 @@ void HELPER(sic)(CPUS390XState *env, uint64_t r1, uint64_t r3)
     qemu_mutex_unlock_iothread();
     /* css_do_sic() may actually return a PGM_xxx value to inject */
     if (r) {
-        s390_program_interrupt(env, -r, GETPC());
+        tcg_s390_program_interrupt(env, -r, GETPC());
     }
 }
 
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 17/31] target/s390x: Push trigger_pgm_exception lower in s390_cpu_tlb_fill
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (15 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 16/31] target/s390x: Use tcg_s390_program_interrupt in TCG helpers David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 18/31] target/s390x: Handle tec " David Hildenbrand
                   ` (15 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Delay triggering an exception until the end, after we have
determined ultimate success or failure, and also taken into
account whether this is a non-faulting probe.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-6-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index dbff772d34..552098be5f 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -127,7 +127,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     CPUS390XState *env = &cpu->env;
     target_ulong vaddr, raddr;
     uint64_t asc;
-    int prot, fail;
+    int prot, fail, excp;
 
     qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",
                   __func__, address, access_type, mmu_idx);
@@ -141,12 +141,14 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
             vaddr &= 0x7fffffff;
         }
         fail = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, true);
+        excp = 0; /* exception already raised */
     } else if (mmu_idx == MMU_REAL_IDX) {
         /* 31-Bit mode */
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
         fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot);
+        excp = 0; /* exception already raised */
     } else {
         g_assert_not_reached();
     }
@@ -159,7 +161,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         qemu_log_mask(CPU_LOG_MMU,
                       "%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n",
                       __func__, (uint64_t)raddr, (uint64_t)ram_size);
-        trigger_pgm_exception(env, PGM_ADDRESSING, ILEN_AUTO);
+        excp = PGM_ADDRESSING;
         fail = 1;
     }
 
@@ -175,6 +177,9 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         return false;
     }
 
+    if (excp) {
+        trigger_pgm_exception(env, excp, ILEN_AUTO);
+    }
     cpu_restore_state(cs, retaddr, true);
 
     /*
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 18/31] target/s390x: Handle tec in s390_cpu_tlb_fill
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (16 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 17/31] target/s390x: Push trigger_pgm_exception lower in s390_cpu_tlb_fill David Hildenbrand
@ 2019-10-10 11:33 ` " David Hildenbrand
  2019-10-10 11:33 ` [PULL 19/31] target/s390x: Return exception from mmu_translate_real David Hildenbrand
                   ` (14 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

As a step toward moving all excption handling out of mmu_translate,
copy handling of the LowCore tec value from trigger_access_exception
into s390_cpu_tlb_fill.  So far this new plumbing isn't used.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-7-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 552098be5f..ab2ed47fef 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -126,7 +126,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
     target_ulong vaddr, raddr;
-    uint64_t asc;
+    uint64_t asc, tec;
     int prot, fail, excp;
 
     qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",
@@ -162,6 +162,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       "%s: raddr %" PRIx64 " > ram_size %" PRIx64 "\n",
                       __func__, (uint64_t)raddr, (uint64_t)ram_size);
         excp = PGM_ADDRESSING;
+        tec = 0; /* unused */
         fail = 1;
     }
 
@@ -178,6 +179,10 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     }
 
     if (excp) {
+        if (excp != PGM_ADDRESSING) {
+            stq_phys(env_cpu(env)->as,
+                     env->psa + offsetof(LowCore, trans_exc_code), tec);
+        }
         trigger_pgm_exception(env, excp, ILEN_AUTO);
     }
     cpu_restore_state(cs, retaddr, true);
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 19/31] target/s390x: Return exception from mmu_translate_real
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (17 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 18/31] target/s390x: Handle tec " David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 20/31] target/s390x: Remove exc argument to mmu_translate_asce David Hildenbrand
                   ` (13 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Do not raise the exception directly within mmu_translate_real,
but pass it back so that caller may do so.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-8-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c |  4 ++--
 target/s390x/internal.h    |  2 +-
 target/s390x/mmu_helper.c  | 14 ++++++--------
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index ab2ed47fef..906b87c071 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -147,8 +147,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
-        fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot);
-        excp = 0; /* exception already raised */
+        excp = mmu_translate_real(env, vaddr, access_type, &raddr, &prot, &tec);
+        fail = excp;
     } else {
         g_assert_not_reached();
     }
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index c243fa725b..c4388aaf23 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -362,7 +362,7 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
                   target_ulong *raddr, int *flags, bool exc);
 int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
-                       target_ulong *addr, int *flags);
+                       target_ulong *addr, int *flags, uint64_t *tec);
 
 
 /* misc_helper.c */
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 4a794dadcf..e8281d4413 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -554,14 +554,11 @@ void s390_cpu_virt_mem_handle_exc(S390CPU *cpu, uintptr_t ra)
  * @param rw     0 = read, 1 = write, 2 = code fetch
  * @param addr   the translated address is stored to this pointer
  * @param flags  the PAGE_READ/WRITE/EXEC flags are stored to this pointer
- * @return       0 if the translation was successful, < 0 if a fault occurred
+ * @return       0 = success, != 0, the exception to raise
  */
 int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
-                       target_ulong *addr, int *flags)
+                       target_ulong *addr, int *flags, uint64_t *tec)
 {
-    /* Code accesses have an undefined ilc, let's use 2 bytes. */
-    uint64_t tec = (raddr & TARGET_PAGE_MASK) |
-                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     const bool lowprot_enabled = env->cregs[0] & CR0_LOWPROT;
 
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
@@ -570,9 +567,10 @@ int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(raddr) && rw == MMU_DATA_STORE) {
             /* LAP sets bit 56 */
-            tec |= 0x80;
-            trigger_access_exception(env, PGM_PROTECTION, ILEN_AUTO, tec);
-            return -EACCES;
+            *tec = (raddr & TARGET_PAGE_MASK)
+                 | (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ)
+                 | 0x80;
+            return PGM_PROTECTION;
         }
     }
 
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 20/31] target/s390x: Remove exc argument to mmu_translate_asce
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (18 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 19/31] target/s390x: Return exception from mmu_translate_real David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 21/31] target/s390x: Return exception from mmu_translate David Hildenbrand
                   ` (12 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Now that mmu_translate_asce returns the exception instead of
raising it, the argument is unused.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-9-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index e8281d4413..e57d762f45 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -116,7 +116,7 @@ static inline bool read_table_entry(CPUS390XState *env, hwaddr gaddr,
 
 static int mmu_translate_asce(CPUS390XState *env, target_ulong vaddr,
                               uint64_t asc, uint64_t asce, target_ulong *raddr,
-                              int *flags, int rw, bool exc)
+                              int *flags, int rw)
 {
     const bool edat1 = (env->cregs[0] & CR0_EDAT) &&
                        s390_has_feat(S390_FEAT_EDAT);
@@ -423,7 +423,7 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
     }
 
     /* perform the DAT translation */
-    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw, exc);
+    r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw);
     if (unlikely(r)) {
         if (exc) {
             trigger_access_exception(env, r, ilen, tec);
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 21/31] target/s390x: Return exception from mmu_translate
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (19 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 20/31] target/s390x: Remove exc argument to mmu_translate_asce David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 22/31] target/s390x: Return exception from translate_pages David Hildenbrand
                   ` (11 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Do not raise the exception directly within mmu_translate,
but pass it back so that caller may do so.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-10-richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c |  4 +--
 target/s390x/helper.c      |  3 ++-
 target/s390x/internal.h    |  2 +-
 target/s390x/mem_helper.c  | 13 +++++++---
 target/s390x/mmu_helper.c  | 51 +++++++++++++++-----------------------
 5 files changed, 35 insertions(+), 38 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 906b87c071..6a0728b65f 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -140,8 +140,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
-        fail = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, true);
-        excp = 0; /* exception already raised */
+        excp = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, &tec);
+        fail = excp;
     } else if (mmu_idx == MMU_REAL_IDX) {
         /* 31-Bit mode */
         if (!(env->psw.mask & PSW_MASK_64)) {
diff --git a/target/s390x/helper.c b/target/s390x/helper.c
index bf503b56ee..a3a49164e4 100644
--- a/target/s390x/helper.c
+++ b/target/s390x/helper.c
@@ -52,6 +52,7 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
     target_ulong raddr;
     int prot;
     uint64_t asc = env->psw.mask & PSW_MASK_ASC;
+    uint64_t tec;
 
     /* 31-Bit mode */
     if (!(env->psw.mask & PSW_MASK_64)) {
@@ -67,7 +68,7 @@ hwaddr s390_cpu_get_phys_page_debug(CPUState *cs, vaddr vaddr)
      * We want to read code even if IEP is active. Use MMU_DATA_LOAD instead
      * of MMU_INST_FETCH.
      */
-    if (mmu_translate(env, vaddr, MMU_DATA_LOAD, asc, &raddr, &prot, false)) {
+    if (mmu_translate(env, vaddr, MMU_DATA_LOAD, asc, &raddr, &prot, &tec)) {
         return -1;
     }
     return raddr;
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index c4388aaf23..c993c3ef40 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -360,7 +360,7 @@ void probe_write_access(CPUS390XState *env, uint64_t addr, uint64_t len,
 
 /* mmu_helper.c */
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
-                  target_ulong *raddr, int *flags, bool exc);
+                  target_ulong *raddr, int *flags, uint64_t *tec);
 int mmu_translate_real(CPUS390XState *env, target_ulong raddr, int rw,
                        target_ulong *addr, int *flags, uint64_t *tec);
 
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 7d2a652823..e15aa296dd 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2364,8 +2364,8 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
     CPUState *cs = env_cpu(env);
     uint32_t cc = 0;
     uint64_t asc = env->psw.mask & PSW_MASK_ASC;
-    uint64_t ret;
-    int old_exc, flags;
+    uint64_t ret, tec;
+    int old_exc, flags, exc;
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
@@ -2373,7 +2373,14 @@ uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
     }
 
     old_exc = cs->exception_index;
-    if (mmu_translate(env, addr, 0, asc, &ret, &flags, true)) {
+    exc = mmu_translate(env, addr, 0, asc, &ret, &flags, &tec);
+    if (exc) {
+        /*
+         * We don't care about ILEN or TEC, as we're not going to
+         * deliver the exception -- thus resetting exception_index below.
+         * TODO: clean this up.
+         */
+        trigger_pgm_exception(env, exc, ILEN_UNWIND);
         cc = 3;
     }
     if (cs->exception_index == EXCP_PGM) {
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index e57d762f45..001d0a9c8a 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -365,20 +365,18 @@ static void mmu_handle_skey(target_ulong addr, int rw, int *flags)
  * @param raddr  the translated address is stored to this pointer
  * @param flags  the PAGE_READ/WRITE/EXEC flags are stored to this pointer
  * @param exc    true = inject a program check if a fault occurred
- * @return       0 if the translation was successful, -1 if a fault occurred
+ * @return       0 = success, != 0, the exception to raise
  */
 int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
-                  target_ulong *raddr, int *flags, bool exc)
+                  target_ulong *raddr, int *flags, uint64_t *tec)
 {
-    /* Code accesses have an undefined ilc, let's use 2 bytes. */
-    const int ilen = (rw == MMU_INST_FETCH) ? 2 : ILEN_AUTO;
-    uint64_t tec = (vaddr & TARGET_PAGE_MASK) | (asc >> 46) |
-                   (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     uint64_t asce;
     int r;
 
-
+    *tec = (vaddr & TARGET_PAGE_MASK) | (asc >> 46) |
+            (rw == MMU_DATA_STORE ? FS_WRITE : FS_READ);
     *flags = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+
     if (is_low_address(vaddr & TARGET_PAGE_MASK) && lowprot_enabled(env, asc)) {
         /*
          * If any part of this page is currently protected, make sure the
@@ -390,12 +388,9 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
          */
         *flags |= PAGE_WRITE_INV;
         if (is_low_address(vaddr) && rw == MMU_DATA_STORE) {
-            if (exc) {
-                /* LAP sets bit 56 */
-                tec |= 0x80;
-                trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
-            }
-            return -EACCES;
+            /* LAP sets bit 56 */
+            *tec |= 0x80;
+            return PGM_PROTECTION;
         }
     }
 
@@ -425,30 +420,21 @@ int mmu_translate(CPUS390XState *env, target_ulong vaddr, int rw, uint64_t asc,
     /* perform the DAT translation */
     r = mmu_translate_asce(env, vaddr, asc, asce, raddr, flags, rw);
     if (unlikely(r)) {
-        if (exc) {
-            trigger_access_exception(env, r, ilen, tec);
-        }
-        return -1;
+        return r;
     }
 
     /* check for DAT protection */
     if (unlikely(rw == MMU_DATA_STORE && !(*flags & PAGE_WRITE))) {
-        if (exc) {
-            /* DAT sets bit 61 only */
-            tec |= 0x4;
-            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
-        }
-        return -1;
+        /* DAT sets bit 61 only */
+        *tec |= 0x4;
+        return PGM_PROTECTION;
     }
 
     /* check for Instruction-Execution-Protection */
     if (unlikely(rw == MMU_INST_FETCH && !(*flags & PAGE_EXEC))) {
-        if (exc) {
-            /* IEP sets bit 56 and 61 */
-            tec |= 0x84;
-            trigger_access_exception(env, PGM_PROTECTION, ilen, tec);
-        }
-        return -1;
+        /* IEP sets bit 56 and 61 */
+        *tec |= 0x84;
+        return PGM_PROTECTION;
     }
 
 nodat:
@@ -472,9 +458,12 @@ static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
     int ret, i, pflags;
 
     for (i = 0; i < nr_pages; i++) {
-        ret = mmu_translate(env, addr, is_write, asc, &pages[i], &pflags, true);
+        uint64_t tec;
+
+        ret = mmu_translate(env, addr, is_write, asc, &pages[i], &pflags, &tec);
         if (ret) {
-            return ret;
+            trigger_access_exception(env, ret, ILEN_AUTO, tec);
+            return -EFAULT;
         }
         if (!address_space_access_valid(&address_space_memory, pages[i],
                                         TARGET_PAGE_SIZE, is_write,
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 22/31] target/s390x: Return exception from translate_pages
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (20 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 21/31] target/s390x: Return exception from mmu_translate David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 23/31] target/s390x: Remove fail variable from s390_cpu_tlb_fill David Hildenbrand
                   ` (10 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Do not raise the exception directly within translate_pages,
but pass it back so that caller may do so.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-11-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 001d0a9c8a..869debd30a 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -451,25 +451,22 @@ nodat:
  * the MEMOP interface.
  */
 static int translate_pages(S390CPU *cpu, vaddr addr, int nr_pages,
-                           target_ulong *pages, bool is_write)
+                           target_ulong *pages, bool is_write, uint64_t *tec)
 {
     uint64_t asc = cpu->env.psw.mask & PSW_MASK_ASC;
     CPUS390XState *env = &cpu->env;
     int ret, i, pflags;
 
     for (i = 0; i < nr_pages; i++) {
-        uint64_t tec;
-
-        ret = mmu_translate(env, addr, is_write, asc, &pages[i], &pflags, &tec);
+        ret = mmu_translate(env, addr, is_write, asc, &pages[i], &pflags, tec);
         if (ret) {
-            trigger_access_exception(env, ret, ILEN_AUTO, tec);
-            return -EFAULT;
+            return ret;
         }
         if (!address_space_access_valid(&address_space_memory, pages[i],
                                         TARGET_PAGE_SIZE, is_write,
                                         MEMTXATTRS_UNSPECIFIED)) {
-            trigger_access_exception(env, PGM_ADDRESSING, ILEN_AUTO, 0);
-            return -EFAULT;
+            *tec = 0; /* unused */
+            return PGM_ADDRESSING;
         }
         addr += TARGET_PAGE_SIZE;
     }
@@ -497,6 +494,7 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
 {
     int currlen, nr_pages, i;
     target_ulong *pages;
+    uint64_t tec;
     int ret;
 
     if (kvm_enabled()) {
@@ -510,8 +508,10 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
                + 1;
     pages = g_malloc(nr_pages * sizeof(*pages));
 
-    ret = translate_pages(cpu, laddr, nr_pages, pages, is_write);
-    if (ret == 0 && hostbuf != NULL) {
+    ret = translate_pages(cpu, laddr, nr_pages, pages, is_write, &tec);
+    if (ret) {
+        trigger_access_exception(&cpu->env, ret, ILEN_AUTO, tec);
+    } else if (hostbuf != NULL) {
         /* Copy data by stepping through the area page by page */
         for (i = 0; i < nr_pages; i++) {
             currlen = MIN(len, TARGET_PAGE_SIZE - (laddr % TARGET_PAGE_SIZE));
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 23/31] target/s390x: Remove fail variable from s390_cpu_tlb_fill
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (21 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 22/31] target/s390x: Return exception from translate_pages David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 24/31] target/s390x: Simplify helper_lra David Hildenbrand
                   ` (9 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

Now that excp always contains a real exception number, we can
use that instead of a separate fail variable.  This allows a
redundant test to be removed.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-12-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 19 +++++++------------
 1 file changed, 7 insertions(+), 12 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 6a0728b65f..98a1ee8317 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -127,7 +127,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
     CPUS390XState *env = &cpu->env;
     target_ulong vaddr, raddr;
     uint64_t asc, tec;
-    int prot, fail, excp;
+    int prot, excp;
 
     qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",
                   __func__, address, access_type, mmu_idx);
@@ -141,20 +141,18 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
             vaddr &= 0x7fffffff;
         }
         excp = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, &tec);
-        fail = excp;
     } else if (mmu_idx == MMU_REAL_IDX) {
         /* 31-Bit mode */
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
         excp = mmu_translate_real(env, vaddr, access_type, &raddr, &prot, &tec);
-        fail = excp;
     } else {
         g_assert_not_reached();
     }
 
     /* check out of RAM access */
-    if (!fail &&
+    if (!excp &&
         !address_space_access_valid(&address_space_memory, raddr,
                                     TARGET_PAGE_SIZE, access_type,
                                     MEMTXATTRS_UNSPECIFIED)) {
@@ -163,10 +161,9 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
                       __func__, (uint64_t)raddr, (uint64_t)ram_size);
         excp = PGM_ADDRESSING;
         tec = 0; /* unused */
-        fail = 1;
     }
 
-    if (!fail) {
+    if (!excp) {
         qemu_log_mask(CPU_LOG_MMU,
                       "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
                       __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);
@@ -178,13 +175,11 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         return false;
     }
 
-    if (excp) {
-        if (excp != PGM_ADDRESSING) {
-            stq_phys(env_cpu(env)->as,
-                     env->psa + offsetof(LowCore, trans_exc_code), tec);
-        }
-        trigger_pgm_exception(env, excp, ILEN_AUTO);
+    if (excp != PGM_ADDRESSING) {
+        stq_phys(env_cpu(env)->as,
+                 env->psa + offsetof(LowCore, trans_exc_code), tec);
     }
+    trigger_pgm_exception(env, excp, ILEN_AUTO);
     cpu_restore_state(cs, retaddr, true);
 
     /*
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 24/31] target/s390x: Simplify helper_lra
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (22 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 23/31] target/s390x: Remove fail variable from s390_cpu_tlb_fill David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 25/31] target/s390x: Rely on unwinding in s390_cpu_tlb_fill David Hildenbrand
                   ` (8 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

We currently call trigger_pgm_exception to set cs->exception_index
and env->int_pgm_code and then read the values back and then
reset cs->exception_index so that the exception is not delivered.

Instead, use the exception type that we already have directly
without ever triggering an exception that must be suppressed.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-13-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mem_helper.c | 17 +++--------------
 1 file changed, 3 insertions(+), 14 deletions(-)

diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index e15aa296dd..4254548935 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -2361,34 +2361,23 @@ void HELPER(sturg)(CPUS390XState *env, uint64_t addr, uint64_t v1)
 /* load real address */
 uint64_t HELPER(lra)(CPUS390XState *env, uint64_t addr)
 {
-    CPUState *cs = env_cpu(env);
-    uint32_t cc = 0;
     uint64_t asc = env->psw.mask & PSW_MASK_ASC;
     uint64_t ret, tec;
-    int old_exc, flags, exc;
+    int flags, exc, cc;
 
     /* XXX incomplete - has more corner cases */
     if (!(env->psw.mask & PSW_MASK_64) && (addr >> 32)) {
         tcg_s390_program_interrupt(env, PGM_SPECIAL_OP, GETPC());
     }
 
-    old_exc = cs->exception_index;
     exc = mmu_translate(env, addr, 0, asc, &ret, &flags, &tec);
     if (exc) {
-        /*
-         * We don't care about ILEN or TEC, as we're not going to
-         * deliver the exception -- thus resetting exception_index below.
-         * TODO: clean this up.
-         */
-        trigger_pgm_exception(env, exc, ILEN_UNWIND);
         cc = 3;
-    }
-    if (cs->exception_index == EXCP_PGM) {
-        ret = env->int_pgm_code | 0x80000000;
+        ret = exc | 0x80000000;
     } else {
+        cc = 0;
         ret |= addr & ~TARGET_PAGE_MASK;
     }
-    cs->exception_index = old_exc;
 
     env->cc_op = cc;
     return ret;
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 25/31] target/s390x: Rely on unwinding in s390_cpu_tlb_fill
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (23 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 24/31] target/s390x: Simplify helper_lra David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 26/31] target/s390x: Rely on unwinding in s390_cpu_virt_mem_rw David Hildenbrand
                   ` (7 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

We currently set ilen to AUTO, then overwrite that during
unwinding, then overwrite that for the code access case.

This can be simplified to setting ilen to our arbitrary
value for the (undefined) code access case, then rely on
unwinding to overwrite that with the correct value for
the data access case.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-14-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 23 +++++++----------------
 1 file changed, 7 insertions(+), 16 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 98a1ee8317..8ce992e639 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -96,7 +96,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 {
     S390CPU *cpu = S390_CPU(cs);
 
-    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_AUTO);
+    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_UNWIND);
     /* On real machines this value is dropped into LowMem.  Since this
        is userland, simply put this someplace that cpu_loop can find it.  */
     cpu->env.__excp_addr = address;
@@ -179,24 +179,15 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
         stq_phys(env_cpu(env)->as,
                  env->psa + offsetof(LowCore, trans_exc_code), tec);
     }
-    trigger_pgm_exception(env, excp, ILEN_AUTO);
-    cpu_restore_state(cs, retaddr, true);
 
     /*
-     * The ILC value for code accesses is undefined.  The important
-     * thing here is to *not* leave env->int_pgm_ilen set to ILEN_AUTO,
-     * which would cause do_program_interrupt to attempt to read from
-     * env->psw.addr again.  C.f. the condition in trigger_page_fault,
-     * but is not universally applied.
-     *
-     * ??? If we remove ILEN_AUTO, by moving the computation of ILEN
-     * into cpu_restore_state, then we may remove this entirely.
+     * For data accesses, ILEN will be filled in from the unwind info,
+     * within cpu_loop_exit_restore.  For code accesses, retaddr == 0,
+     * and so unwinding will not occur.  However, ILEN is also undefined
+     * for that case -- we choose to set ILEN = 2.
      */
-    if (access_type == MMU_INST_FETCH) {
-        env->int_pgm_ilen = 2;
-    }
-
-    cpu_loop_exit(cs);
+    trigger_pgm_exception(env, excp, 2);
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 static void do_program_interrupt(CPUS390XState *env)
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 26/31] target/s390x: Rely on unwinding in s390_cpu_virt_mem_rw
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (24 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 25/31] target/s390x: Rely on unwinding in s390_cpu_tlb_fill David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 27/31] target/s390x: Remove ILEN_AUTO David Hildenbrand
                   ` (6 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

For TCG, we will always call s390_cpu_virt_mem_handle_exc,
which will go through the unwinder to set ILEN.  For KVM,
we do not go through do_program_interrupt, so this argument
is unused.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-15-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 869debd30a..df58fb73ba 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -510,7 +510,7 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
 
     ret = translate_pages(cpu, laddr, nr_pages, pages, is_write, &tec);
     if (ret) {
-        trigger_access_exception(&cpu->env, ret, ILEN_AUTO, tec);
+        trigger_access_exception(&cpu->env, ret, ILEN_UNWIND, tec);
     } else if (hostbuf != NULL) {
         /* Copy data by stepping through the area page by page */
         for (i = 0; i < nr_pages; i++) {
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 27/31] target/s390x: Remove ILEN_AUTO
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (25 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 26/31] target/s390x: Rely on unwinding in s390_cpu_virt_mem_rw David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 28/31] target/s390x: Remove ilen argument from trigger_access_exception David Hildenbrand
                   ` (5 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

This setting is no longer used.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-16-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h         | 2 --
 target/s390x/excp_helper.c | 3 ---
 2 files changed, 5 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index 18e7091763..e7a9a58e0f 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -806,8 +806,6 @@ void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
                        uint32_t io_int_parm, uint32_t io_int_word);
 /* instruction length set by unwind info */
 #define ILEN_UNWIND                 0
-/* automatically detect the instruction length */
-#define ILEN_AUTO                   0xff
 #define RA_IGNORED                  0
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index 8ce992e639..c252e9a7d8 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -196,9 +196,6 @@ static void do_program_interrupt(CPUS390XState *env)
     LowCore *lowcore;
     int ilen = env->int_pgm_ilen;
 
-    if (ilen == ILEN_AUTO) {
-        ilen = get_ilen(cpu_ldub_code(env, env->psw.addr));
-    }
     assert(ilen == 2 || ilen == 4 || ilen == 6);
 
     switch (env->int_pgm_code) {
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 28/31] target/s390x: Remove ilen argument from trigger_access_exception
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (26 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 27/31] target/s390x: Remove ILEN_AUTO David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 29/31] target/s390x: Remove ilen argument from trigger_pgm_exception David Hildenbrand
                   ` (4 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

The single caller passes ILEN_UNWIND; pass that along to
trigger_pgm_exception directly.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-17-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/mmu_helper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index df58fb73ba..09c74f17dd 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -33,7 +33,7 @@
 #define FS_WRITE 0x400
 
 static void trigger_access_exception(CPUS390XState *env, uint32_t type,
-                                     uint32_t ilen, uint64_t tec)
+                                     uint64_t tec)
 {
     S390CPU *cpu = env_archcpu(env);
 
@@ -44,7 +44,7 @@ static void trigger_access_exception(CPUS390XState *env, uint32_t type,
         if (type != PGM_ADDRESSING) {
             stq_phys(cs->as, env->psa + offsetof(LowCore, trans_exc_code), tec);
         }
-        trigger_pgm_exception(env, type, ilen);
+        trigger_pgm_exception(env, type, ILEN_UNWIND);
     }
 }
 
@@ -510,7 +510,7 @@ int s390_cpu_virt_mem_rw(S390CPU *cpu, vaddr laddr, uint8_t ar, void *hostbuf,
 
     ret = translate_pages(cpu, laddr, nr_pages, pages, is_write, &tec);
     if (ret) {
-        trigger_access_exception(&cpu->env, ret, ILEN_UNWIND, tec);
+        trigger_access_exception(&cpu->env, ret, tec);
     } else if (hostbuf != NULL) {
         /* Copy data by stepping through the area page by page */
         for (i = 0; i < nr_pages; i++) {
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 29/31] target/s390x: Remove ilen argument from trigger_pgm_exception
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (27 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 28/31] target/s390x: Remove ilen argument from trigger_access_exception David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 30/31] target/s390x: Remove ILEN_UNWIND David Hildenbrand
                   ` (3 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

All but one caller passes ILEN_UNWIND, which is not stored.
For the one use case in s390_cpu_tlb_fill, set int_pgm_ilen
directly, simply to avoid the assert within do_program_interrupt.

Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-18-richard.henderson@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/excp_helper.c | 7 ++++---
 target/s390x/internal.h    | 2 +-
 target/s390x/interrupt.c   | 7 ++-----
 target/s390x/mmu_helper.c  | 2 +-
 4 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index c252e9a7d8..e70c20d363 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -42,7 +42,7 @@ void QEMU_NORETURN tcg_s390_program_interrupt(CPUS390XState *env,
     cpu_restore_state(cs, ra, true);
     qemu_log_mask(CPU_LOG_INT, "program interrupt at %#" PRIx64 "\n",
                   env->psw.addr);
-    trigger_pgm_exception(env, code, ILEN_UNWIND);
+    trigger_pgm_exception(env, code);
     cpu_loop_exit(cs);
 }
 
@@ -96,7 +96,7 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
 {
     S390CPU *cpu = S390_CPU(cs);
 
-    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING, ILEN_UNWIND);
+    trigger_pgm_exception(&cpu->env, PGM_ADDRESSING);
     /* On real machines this value is dropped into LowMem.  Since this
        is userland, simply put this someplace that cpu_loop can find it.  */
     cpu->env.__excp_addr = address;
@@ -186,7 +186,8 @@ bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
      * and so unwinding will not occur.  However, ILEN is also undefined
      * for that case -- we choose to set ILEN = 2.
      */
-    trigger_pgm_exception(env, excp, 2);
+    env->int_pgm_ilen = 2;
+    trigger_pgm_exception(env, excp);
     cpu_loop_exit_restore(cs, retaddr);
 }
 
diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index c993c3ef40..d37816104d 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -317,7 +317,7 @@ void cpu_unmap_lowcore(LowCore *lowcore);
 
 
 /* interrupt.c */
-void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen);
+void trigger_pgm_exception(CPUS390XState *env, uint32_t code);
 void cpu_inject_clock_comparator(S390CPU *cpu);
 void cpu_inject_cpu_timer(S390CPU *cpu);
 void cpu_inject_emergency_signal(S390CPU *cpu, uint16_t src_cpu_addr);
diff --git a/target/s390x/interrupt.c b/target/s390x/interrupt.c
index 2b71e03914..4cdbbc8849 100644
--- a/target/s390x/interrupt.c
+++ b/target/s390x/interrupt.c
@@ -22,16 +22,13 @@
 #endif
 
 /* Ensure to exit the TB after this call! */
-void trigger_pgm_exception(CPUS390XState *env, uint32_t code, uint32_t ilen)
+void trigger_pgm_exception(CPUS390XState *env, uint32_t code)
 {
     CPUState *cs = env_cpu(env);
 
     cs->exception_index = EXCP_PGM;
     env->int_pgm_code = code;
-    /* If ILEN_UNWIND, int_pgm_ilen already has the correct value.  */
-    if (ilen != ILEN_UNWIND) {
-        env->int_pgm_ilen = ilen;
-    }
+    /* env->int_pgm_ilen is already set, or will be set during unwinding */
 }
 
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra)
diff --git a/target/s390x/mmu_helper.c b/target/s390x/mmu_helper.c
index 09c74f17dd..90b81335f9 100644
--- a/target/s390x/mmu_helper.c
+++ b/target/s390x/mmu_helper.c
@@ -44,7 +44,7 @@ static void trigger_access_exception(CPUS390XState *env, uint32_t type,
         if (type != PGM_ADDRESSING) {
             stq_phys(cs->as, env->psa + offsetof(LowCore, trans_exc_code), tec);
         }
-        trigger_pgm_exception(env, type, ILEN_UNWIND);
+        trigger_pgm_exception(env, type);
     }
 }
 
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 30/31] target/s390x: Remove ILEN_UNWIND
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (28 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 29/31] target/s390x: Remove ilen argument from trigger_pgm_exception David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 11:33 ` [PULL 31/31] s390x/tcg: MVCL: Exit to main loop if requested David Hildenbrand
                   ` (2 subsequent siblings)
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: qemu-s390x, Cornelia Huck, Richard Henderson, Thomas Huth,
	David Hildenbrand

From: Richard Henderson <richard.henderson@linaro.org>

This setting is no longer used.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-Id: <20191001171614.8405-19-richard.henderson@linaro.org>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 target/s390x/cpu.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/target/s390x/cpu.h b/target/s390x/cpu.h
index e7a9a58e0f..17460ed7b3 100644
--- a/target/s390x/cpu.h
+++ b/target/s390x/cpu.h
@@ -804,8 +804,6 @@ int cpu_s390x_signal_handler(int host_signum, void *pinfo, void *puc);
 void s390_crw_mchk(void);
 void s390_io_interrupt(uint16_t subchannel_id, uint16_t subchannel_nr,
                        uint32_t io_int_parm, uint32_t io_int_word);
-/* instruction length set by unwind info */
-#define ILEN_UNWIND                 0
 #define RA_IGNORED                  0
 void s390_program_interrupt(CPUS390XState *env, uint32_t code, uintptr_t ra);
 /* service interrupts are floating therefore we must not pass an cpustate */
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PULL 31/31] s390x/tcg: MVCL: Exit to main loop if requested
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (29 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 30/31] target/s390x: Remove ILEN_UNWIND David Hildenbrand
@ 2019-10-10 11:33 ` David Hildenbrand
  2019-10-10 12:33 ` [PULL 00/31] s390x/tcg update no-reply
  2019-10-14 11:26 ` Peter Maydell
  32 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 11:33 UTC (permalink / raw)
  To: Peter Maydell, qemu-devel
  Cc: Thomas Huth, David Hildenbrand, Cornelia Huck, Richard Henderson,
	qemu-s390x, Paolo Bonzini, Alex Bennée

MVCL is interruptible and we should check for interrupts and process
them after writing back the variables to the registers. Let's check
for any exit requests and exit to the main loop. Introduce a new helper
function for that: cpu_loop_exit_requested().

When booting Fedora 30, I can see a handful of these exits and it seems
to work reliable. Also, Richard explained why this works correctly even
when MVCL is called via EXECUTE:

    (1) TB with EXECUTE runs, at address Ae
        - env->psw_addr stored with Ae.
        - helper_ex() runs, memory address Am computed
          from D2a(X2a,B2a) or from psw.addr+RI2.
        - env->ex_value stored with memory value modified by R1a

    (2) TB of executee runs,
        - env->ex_value stored with 0.
        - helper_mvcl() runs, using and updating R1b, R1b+1, R2b, R2b+1.

    (3a) helper_mvcl() completes,
         - TB of executee continues, psw.addr += ilen.
         - Next instruction is the one following EXECUTE.

    (3b) helper_mvcl() exits to main loop,
         - cpu_loop_exit_restore() unwinds psw.addr = Ae.
         - Next instruction is the EXECUTE itself...
         - goto 1.

As the PoP mentiones that an interruptible instruction called via EXECUTE
should avoid modifying storage/registers that are used by EXECUTE itself,
it is fine to retrigger EXECUTE.

Cc: Alex Bennée <alex.bennee@linaro.org>
Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 include/exec/exec-all.h   | 17 +++++++++++++++++
 target/s390x/mem_helper.c | 11 ++++++++++-
 2 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/include/exec/exec-all.h b/include/exec/exec-all.h
index 49db07ba0b..04795c49bf 100644
--- a/include/exec/exec-all.h
+++ b/include/exec/exec-all.h
@@ -72,6 +72,23 @@ void QEMU_NORETURN cpu_loop_exit(CPUState *cpu);
 void QEMU_NORETURN cpu_loop_exit_restore(CPUState *cpu, uintptr_t pc);
 void QEMU_NORETURN cpu_loop_exit_atomic(CPUState *cpu, uintptr_t pc);
 
+/**
+ * cpu_loop_exit_requested:
+ * @cpu: The CPU state to be tested
+ *
+ * Indicate if somebody asked for a return of the CPU to the main loop
+ * (e.g., via cpu_exit() or cpu_interrupt()).
+ *
+ * This is helpful for architectures that support interruptible
+ * instructions. After writing back all state to registers/memory, this
+ * call can be used to check if it makes sense to return to the main loop
+ * or to continue executing the interruptible instruction.
+ */
+static inline bool cpu_loop_exit_requested(CPUState *cpu)
+{
+    return (int32_t)atomic_read(&cpu_neg(cpu)->icount_decr.u32) < 0;
+}
+
 #if !defined(CONFIG_USER_ONLY)
 void cpu_reloading_memory_map(void);
 /**
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 4254548935..2325767f17 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -1015,6 +1015,7 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
     uint64_t srclen = env->regs[r2 + 1] & 0xffffff;
     uint64_t src = get_address(env, r2);
     uint8_t pad = env->regs[r2 + 1] >> 24;
+    CPUState *cs = env_cpu(env);
     S390Access srca, desta;
     uint32_t cc, cur_len;
 
@@ -1065,7 +1066,15 @@ uint32_t HELPER(mvcl)(CPUS390XState *env, uint32_t r1, uint32_t r2)
         env->regs[r1 + 1] = deposit64(env->regs[r1 + 1], 0, 24, destlen);
         set_address_zero(env, r1, dest);
 
-        /* TODO: Deliver interrupts. */
+        /*
+         * MVCL is interruptible. Return to the main loop if requested after
+         * writing back all state to registers. If no interrupt will get
+         * injected, we'll end up back in this handler and continue processing
+         * the remaining parts.
+         */
+        if (destlen && unlikely(cpu_loop_exit_requested(cs))) {
+            cpu_loop_exit_restore(cs, ra);
+        }
     }
     return cc;
 }
-- 
2.21.0



^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PULL 00/31] s390x/tcg update
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (30 preceding siblings ...)
  2019-10-10 11:33 ` [PULL 31/31] s390x/tcg: MVCL: Exit to main loop if requested David Hildenbrand
@ 2019-10-10 12:33 ` no-reply
  2019-10-10 13:11   ` David Hildenbrand
  2019-10-14 11:26 ` Peter Maydell
  32 siblings, 1 reply; 35+ messages in thread
From: no-reply @ 2019-10-10 12:33 UTC (permalink / raw)
  To: david
  Cc: peter.maydell, thuth, david, cohuck, richard.henderson,
	qemu-devel, qemu-s390x

Patchew URL: https://patchew.org/QEMU/20191010113356.5017-1-david@redhat.com/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      stubs/pc_madt_cpu_entry.o
  CC      stubs/vmgenid.o
  CC      stubs/xen-common.o
make: *** [Makefile:994: docs/interop/index.html] Error 2
make: *** Deleting file 'docs/interop/index.html'
make: *** Waiting for unfinished jobs....
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ac34ecad3e534df8b0eacbb2152563e8', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-wk1otdub/src/docker-src.2019-10-10-08.31.07.10413:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=ac34ecad3e534df8b0eacbb2152563e8
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-wk1otdub/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m12.498s
user    0m6.664s


The full log is available at
http://patchew.org/logs/20191010113356.5017-1-david@redhat.com/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PULL 00/31] s390x/tcg update
  2019-10-10 12:33 ` [PULL 00/31] s390x/tcg update no-reply
@ 2019-10-10 13:11   ` David Hildenbrand
  0 siblings, 0 replies; 35+ messages in thread
From: David Hildenbrand @ 2019-10-10 13:11 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-s390x, cohuck, richard.henderson, thuth

On 10.10.19 14:33, no-reply@patchew.org wrote:
> Patchew URL: https://patchew.org/QEMU/20191010113356.5017-1-david@redhat.com/
> 
> 
> 
> Hi,
> 
> This series failed the docker-mingw@fedora build test. Please find the testing commands and
> their output below. If you have Docker installed, you can probably reproduce it
> locally.
> 
> === TEST SCRIPT BEGIN ===
> #! /bin/bash
> export ARCH=x86_64
> make docker-image-fedora V=1 NETWORK=1
> time make docker-test-mingw@fedora J=14 NETWORK=1
> === TEST SCRIPT END ===
> 
>   CC      stubs/pc_madt_cpu_entry.o
>   CC      stubs/vmgenid.o
>   CC      stubs/xen-common.o
> make: *** [Makefile:994: docs/interop/index.html] Error 2
> make: *** Deleting file 'docs/interop/index.html'
> make: *** Waiting for unfinished jobs....
> Traceback (most recent call last):
> ---
>     raise CalledProcessError(retcode, cmd)
> subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=ac34ecad3e534df8b0eacbb2152563e8', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-wk1otdub/src/docker-src.2019-10-10-08.31.07.10413:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
> filter=--filter=label=com.qemu.instance.uuid=ac34ecad3e534df8b0eacbb2152563e8
> make[1]: *** [docker-run] Error 1
> make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-wk1otdub/src'
> make: *** [docker-run-test-mingw@fedora] Error 2
> 
> real    2m12.498s
> user    0m6.664s

False negative, just tried to reproduce and the build succeeded without
an issue.

rm -f qapi-gen-timestamp
rm -rf qga/qapi-generated
rm -f config-all-devices.mak
    CLEANUP /home/dhildenb/git/qemu/docker-src.2019-10-10-15.06.03.19743
make[1]: Verzeichnis „/home/dhildenb/git/qemu“ wird verlassen

real    3m30,648s
user    0m1,855s
sys     0m0,885s


-- 

Thanks,

David / dhildenb


^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PULL 00/31] s390x/tcg update
  2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
                   ` (31 preceding siblings ...)
  2019-10-10 12:33 ` [PULL 00/31] s390x/tcg update no-reply
@ 2019-10-14 11:26 ` Peter Maydell
  32 siblings, 0 replies; 35+ messages in thread
From: Peter Maydell @ 2019-10-14 11:26 UTC (permalink / raw)
  To: David Hildenbrand
  Cc: Thomas Huth, qemu-s390x, Cornelia Huck, Richard Henderson,
	QEMU Developers

On Thu, 10 Oct 2019 at 12:34, David Hildenbrand <david@redhat.com> wrote:
>
> Hi Peter,
>
> The following changes since commit 98b2e3c9ab3abfe476a2b02f8f51813edb90e72d:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-10-08 16:08:35 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/davidhildenbrand/qemu.git tags/s390x-tcg-2019-10-10
>
> for you to fetch changes up to 1f6493be088da969efeeee0100ea4330fb804cdf:
>
>   s390x/tcg: MVCL: Exit to main loop if requested (2019-10-10 12:27:15 +0200)
>
> ----------------------------------------------------------------
> - MMU DAT translation rewrite and cleanup
> - Implement more TCG CPU features related to the MMU (e.g., IEP)
> - Add the current instruction length to unwind data and clean up
> - Resolve one TODO for the MVCL instruction
>


Applied, thanks.

Please update the changelog at https://wiki.qemu.org/ChangeLog/4.2
for any user-visible changes.

-- PMM


^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, back to index

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-10 11:33 [PULL 00/31] s390x/tcg update David Hildenbrand
2019-10-10 11:33 ` [PULL 01/31] s390x/mmu: Drop debug logging from MMU code David Hildenbrand
2019-10-10 11:33 ` [PULL 02/31] s390x/mmu: Move DAT protection handling out of mmu_translate_asce() David Hildenbrand
2019-10-10 11:33 ` [PULL 03/31] s390x/mmu: Inject DAT exceptions from a single place David Hildenbrand
2019-10-10 11:33 ` [PULL 04/31] s390x/mmu: Inject PGM_ADDRESSING on bogus table addresses David Hildenbrand
2019-10-10 11:33 ` [PULL 05/31] s390x/mmu: Use TARGET_PAGE_MASK in mmu_translate_pte() David Hildenbrand
2019-10-10 11:33 ` [PULL 06/31] s390x/mmu: DAT table definition overhaul David Hildenbrand
2019-10-10 11:33 ` [PULL 07/31] s390x/mmu: Convert to non-recursive page table walk David Hildenbrand
2019-10-10 11:33 ` [PULL 08/31] s390x/mmu: Add EDAT2 translation support David Hildenbrand
2019-10-10 11:33 ` [PULL 09/31] s390x/mmu: Implement ESOP-2 and access-exception-fetch/store-indication facility David Hildenbrand
2019-10-10 11:33 ` [PULL 10/31] s390x/mmu: Implement Instruction-Execution-Protection Facility David Hildenbrand
2019-10-10 11:33 ` [PULL 11/31] s390x/cpumodel: Prepare for changes of QEMU model David Hildenbrand
2019-10-10 11:33 ` [PULL 12/31] s390x/cpumodel: Add new TCG features to QEMU cpu model David Hildenbrand
2019-10-10 11:33 ` [PULL 13/31] target/s390x: Add ilen to unwind data David Hildenbrand
2019-10-10 11:33 ` [PULL 14/31] target/s390x: Remove ilen parameter from tcg_s390_program_interrupt David Hildenbrand
2019-10-10 11:33 ` [PULL 15/31] target/s390x: Remove ilen parameter from s390_program_interrupt David Hildenbrand
2019-10-10 11:33 ` [PULL 16/31] target/s390x: Use tcg_s390_program_interrupt in TCG helpers David Hildenbrand
2019-10-10 11:33 ` [PULL 17/31] target/s390x: Push trigger_pgm_exception lower in s390_cpu_tlb_fill David Hildenbrand
2019-10-10 11:33 ` [PULL 18/31] target/s390x: Handle tec " David Hildenbrand
2019-10-10 11:33 ` [PULL 19/31] target/s390x: Return exception from mmu_translate_real David Hildenbrand
2019-10-10 11:33 ` [PULL 20/31] target/s390x: Remove exc argument to mmu_translate_asce David Hildenbrand
2019-10-10 11:33 ` [PULL 21/31] target/s390x: Return exception from mmu_translate David Hildenbrand
2019-10-10 11:33 ` [PULL 22/31] target/s390x: Return exception from translate_pages David Hildenbrand
2019-10-10 11:33 ` [PULL 23/31] target/s390x: Remove fail variable from s390_cpu_tlb_fill David Hildenbrand
2019-10-10 11:33 ` [PULL 24/31] target/s390x: Simplify helper_lra David Hildenbrand
2019-10-10 11:33 ` [PULL 25/31] target/s390x: Rely on unwinding in s390_cpu_tlb_fill David Hildenbrand
2019-10-10 11:33 ` [PULL 26/31] target/s390x: Rely on unwinding in s390_cpu_virt_mem_rw David Hildenbrand
2019-10-10 11:33 ` [PULL 27/31] target/s390x: Remove ILEN_AUTO David Hildenbrand
2019-10-10 11:33 ` [PULL 28/31] target/s390x: Remove ilen argument from trigger_access_exception David Hildenbrand
2019-10-10 11:33 ` [PULL 29/31] target/s390x: Remove ilen argument from trigger_pgm_exception David Hildenbrand
2019-10-10 11:33 ` [PULL 30/31] target/s390x: Remove ILEN_UNWIND David Hildenbrand
2019-10-10 11:33 ` [PULL 31/31] s390x/tcg: MVCL: Exit to main loop if requested David Hildenbrand
2019-10-10 12:33 ` [PULL 00/31] s390x/tcg update no-reply
2019-10-10 13:11   ` David Hildenbrand
2019-10-14 11:26 ` Peter Maydell

QEMU-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/qemu-devel/0 qemu-devel/git/0.git
	git clone --mirror https://lore.kernel.org/qemu-devel/1 qemu-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 qemu-devel qemu-devel/ https://lore.kernel.org/qemu-devel \
		qemu-devel@nongnu.org qemu-devel@archiver.kernel.org
	public-inbox-index qemu-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.nongnu.qemu-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox