diff mbox series

[v2,18/27] target/s390x: Convert to CPUClass::tlb_fill

Message ID 20190509060246.4031-19-richard.henderson@linaro.org
State Superseded
Headers show
Series tcg: Add CPUClass::tlb_fill | expand

Commit Message

Richard Henderson May 9, 2019, 6:02 a.m. UTC 
Cc: qemu-s390x@nongnu.org
Cc: Cornelia Huck <cohuck@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

---
v2: Keep user-only and system tlb_fill separate.
---
 target/s390x/internal.h    |  5 +--
 target/s390x/cpu.c         |  5 ++-
 target/s390x/excp_helper.c | 73 ++++++++++++++++++++++++++------------
 target/s390x/mem_helper.c  | 16 ---------
 4 files changed, 55 insertions(+), 44 deletions(-)

-- 
2.17.1

Comments

David Hildenbrand May 9, 2019, 7:04 a.m. UTC | #1 
On 09.05.19 08:02, Richard Henderson wrote:
> Cc: qemu-s390x@nongnu.org

> Cc: Cornelia Huck <cohuck@redhat.com>

> Cc: David Hildenbrand <david@redhat.com>

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

> ---

> v2: Keep user-only and system tlb_fill separate.

> ---

>  target/s390x/internal.h    |  5 +--

>  target/s390x/cpu.c         |  5 ++-

>  target/s390x/excp_helper.c | 73 ++++++++++++++++++++++++++------------

>  target/s390x/mem_helper.c  | 16 ---------

>  4 files changed, 55 insertions(+), 44 deletions(-)

> 

> diff --git a/target/s390x/internal.h b/target/s390x/internal.h

> index 26575f2130..56534b38e0 100644

> --- a/target/s390x/internal.h

> +++ b/target/s390x/internal.h

> @@ -263,8 +263,9 @@ ObjectClass *s390_cpu_class_by_name(const char *name);

>  void s390x_cpu_debug_excp_handler(CPUState *cs);

>  void s390_cpu_do_interrupt(CPUState *cpu);

>  bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);

> -int s390_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw,

> -                              int mmu_idx);

> +bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> +                       MMUAccessType access_type, int mmu_idx,

> +                       bool probe, uintptr_t retaddr);

>  void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,

>                                     MMUAccessType access_type,

>                                     int mmu_idx, uintptr_t retaddr);

> diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c

> index b58ef0a8ef..e28939032b 100644

> --- a/target/s390x/cpu.c

> +++ b/target/s390x/cpu.c

> @@ -478,9 +478,8 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)

>      cc->set_pc = s390_cpu_set_pc;

>      cc->gdb_read_register = s390_cpu_gdb_read_register;

>      cc->gdb_write_register = s390_cpu_gdb_write_register;

> -#ifdef CONFIG_USER_ONLY

> -    cc->handle_mmu_fault = s390_cpu_handle_mmu_fault;

> -#else

> +    cc->tlb_fill = s390_cpu_tlb_fill;

> +#ifndef CONFIG_USER_ONLY

>      cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;

>      cc->vmsd = &vmstate_s390_cpu;

>      cc->write_elf64_note = s390_cpu_write_elf64_note;

> diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c

> index f84bfb1284..a4e134bcab 100644

> --- a/target/s390x/excp_helper.c

> +++ b/target/s390x/excp_helper.c

> @@ -74,8 +74,9 @@ void s390_cpu_do_interrupt(CPUState *cs)

>      cs->exception_index = -1;

>  }

>  

> -int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,

> -                              int rw, int mmu_idx)

> +bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> +                       MMUAccessType access_type, int mmu_idx,

> +                       bool probe, uintptr_t retaddr)

>  {

>      S390CPU *cpu = S390_CPU(cs);

>  

> @@ -83,7 +84,7 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,

>      /* 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;

> -    return 1;

> +    cpu_loop_exit_restore(cs, retaddr);

>  }

>  

>  #else /* !CONFIG_USER_ONLY */

> @@ -102,19 +103,20 @@ static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)

>      }

>  }

>  

> -int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size,

> -                              int rw, int mmu_idx)

> +bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,

> +                       MMUAccessType access_type, int mmu_idx,

> +                       bool probe, uintptr_t retaddr)

>  {

>      S390CPU *cpu = S390_CPU(cs);

>      CPUS390XState *env = &cpu->env;

>      target_ulong vaddr, raddr;

>      uint64_t asc;

> -    int prot;

> +    int prot, fail;

>  

>      qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",

> -                  __func__, orig_vaddr, rw, mmu_idx);

> +                  __func__, address, access_type, mmu_idx);

>  

> -    vaddr = orig_vaddr;

> +    vaddr = address;

>  

>      if (mmu_idx < MMU_REAL_IDX) {

>          asc = cpu_mmu_idx_to_asc(mmu_idx);

> @@ -122,39 +124,64 @@ int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size,

>          if (!(env->psw.mask & PSW_MASK_64)) {

>              vaddr &= 0x7fffffff;

>          }

> -        if (mmu_translate(env, vaddr, rw, asc, &raddr, &prot, true)) {

> -            return 1;

> -        }

> +        fail = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, true);

>      } else if (mmu_idx == MMU_REAL_IDX) {

>          /* 31-Bit mode */

>          if (!(env->psw.mask & PSW_MASK_64)) {

>              vaddr &= 0x7fffffff;

>          }

> -        if (mmu_translate_real(env, vaddr, rw, &raddr, &prot)) {

> -            return 1;

> -        }

> +        fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot);

>      } else {

> -        abort();

> +        g_assert_not_reached();

>      }

>  

>      /* check out of RAM access */

> -    if (!address_space_access_valid(&address_space_memory, raddr,

> -                                    TARGET_PAGE_SIZE, rw,

> +    if (!fail &&

> +        !address_space_access_valid(&address_space_memory, raddr,

> +                                    TARGET_PAGE_SIZE, access_type,

>                                      MEMTXATTRS_UNSPECIFIED)) {

>          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);

> -        return 1;

> +        fail = 1;

>      }

>  

> -    qemu_log_mask(CPU_LOG_MMU, "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",

> -            __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);

> +    if (!fail) {

> +        qemu_log_mask(CPU_LOG_MMU,

> +                      "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",

> +                      __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);

> +        tlb_set_page(cs, address & TARGET_PAGE_MASK, raddr, prot,

> +                     mmu_idx, TARGET_PAGE_SIZE);

> +        return true;

> +    }

> +    if (probe) {

> +        return false;

> +    }

>  

> -    tlb_set_page(cs, orig_vaddr & TARGET_PAGE_MASK, raddr, prot,

> -                 mmu_idx, TARGET_PAGE_SIZE);

> +    cpu_restore_state(cs, retaddr, true);

>  

> -    return 0;

> +    /*

> +     * 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.

> +     */

> +    if (access_type == MMU_INST_FETCH) {

> +        env->int_pgm_ilen = 2;

> +    }

> +

> +    cpu_loop_exit(cs);

> +}

> +

> +void tlb_fill(CPUState *cs, target_ulong addr, int size,

> +              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)

> +{

> +    s390_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);

>  }

>  

>  static void do_program_interrupt(CPUS390XState *env)

> diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c

> index 3f76a8abfd..ffd5f02fbe 100644

> --- a/target/s390x/mem_helper.c

> +++ b/target/s390x/mem_helper.c

> @@ -33,22 +33,6 @@

>  

>  /*****************************************************************************/

>  /* Softmmu support */

> -#if !defined(CONFIG_USER_ONLY)

> -

> -/* try to fill the TLB and return an exception if error. If retaddr is

> -   NULL, it means that the function was called in C code (i.e. not

> -   from generated code or from helper.c) */

> -/* XXX: fix it to restore all registers */

> -void tlb_fill(CPUState *cs, target_ulong addr, int size,

> -              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)

> -{

> -    int ret = s390_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);

> -    if (unlikely(ret != 0)) {

> -        cpu_loop_exit_restore(cs, retaddr);

> -    }

> -}

> -

> -#endif

>  

>  /* #define DEBUG_HELPER */

>  #ifdef DEBUG_HELPER

> 



Reviewed-by: David Hildenbrand <david@redhat.com>


-- 

Thanks,

David / dhildenb
diff mbox series

Patch

diff --git a/target/s390x/internal.h b/target/s390x/internal.h
index 26575f2130..56534b38e0 100644
--- a/target/s390x/internal.h
+++ b/target/s390x/internal.h
@@ -263,8 +263,9 @@  ObjectClass *s390_cpu_class_by_name(const char *name);
 void s390x_cpu_debug_excp_handler(CPUState *cs);
 void s390_cpu_do_interrupt(CPUState *cpu);
 bool s390_cpu_exec_interrupt(CPUState *cpu, int int_req);
-int s390_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw,
-                              int mmu_idx);
+bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool probe, uintptr_t retaddr);
 void s390x_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                    MMUAccessType access_type,
                                    int mmu_idx, uintptr_t retaddr);
diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c
index b58ef0a8ef..e28939032b 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -478,9 +478,8 @@  static void s390_cpu_class_init(ObjectClass *oc, void *data)
     cc->set_pc = s390_cpu_set_pc;
     cc->gdb_read_register = s390_cpu_gdb_read_register;
     cc->gdb_write_register = s390_cpu_gdb_write_register;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = s390_cpu_handle_mmu_fault;
-#else
+    cc->tlb_fill = s390_cpu_tlb_fill;
+#ifndef CONFIG_USER_ONLY
     cc->get_phys_page_debug = s390_cpu_get_phys_page_debug;
     cc->vmsd = &vmstate_s390_cpu;
     cc->write_elf64_note = s390_cpu_write_elf64_note;
diff --git a/target/s390x/excp_helper.c b/target/s390x/excp_helper.c
index f84bfb1284..a4e134bcab 100644
--- a/target/s390x/excp_helper.c
+++ b/target/s390x/excp_helper.c
@@ -74,8 +74,9 @@  void s390_cpu_do_interrupt(CPUState *cs)
     cs->exception_index = -1;
 }
 
-int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
-                              int rw, int mmu_idx)
+bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool probe, uintptr_t retaddr)
 {
     S390CPU *cpu = S390_CPU(cs);
 
@@ -83,7 +84,7 @@  int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
     /* 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;
-    return 1;
+    cpu_loop_exit_restore(cs, retaddr);
 }
 
 #else /* !CONFIG_USER_ONLY */
@@ -102,19 +103,20 @@  static inline uint64_t cpu_mmu_idx_to_asc(int mmu_idx)
     }
 }
 
-int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size,
-                              int rw, int mmu_idx)
+bool s390_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool probe, uintptr_t retaddr)
 {
     S390CPU *cpu = S390_CPU(cs);
     CPUS390XState *env = &cpu->env;
     target_ulong vaddr, raddr;
     uint64_t asc;
-    int prot;
+    int prot, fail;
 
     qemu_log_mask(CPU_LOG_MMU, "%s: addr 0x%" VADDR_PRIx " rw %d mmu_idx %d\n",
-                  __func__, orig_vaddr, rw, mmu_idx);
+                  __func__, address, access_type, mmu_idx);
 
-    vaddr = orig_vaddr;
+    vaddr = address;
 
     if (mmu_idx < MMU_REAL_IDX) {
         asc = cpu_mmu_idx_to_asc(mmu_idx);
@@ -122,39 +124,64 @@  int s390_cpu_handle_mmu_fault(CPUState *cs, vaddr orig_vaddr, int size,
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
-        if (mmu_translate(env, vaddr, rw, asc, &raddr, &prot, true)) {
-            return 1;
-        }
+        fail = mmu_translate(env, vaddr, access_type, asc, &raddr, &prot, true);
     } else if (mmu_idx == MMU_REAL_IDX) {
         /* 31-Bit mode */
         if (!(env->psw.mask & PSW_MASK_64)) {
             vaddr &= 0x7fffffff;
         }
-        if (mmu_translate_real(env, vaddr, rw, &raddr, &prot)) {
-            return 1;
-        }
+        fail = mmu_translate_real(env, vaddr, access_type, &raddr, &prot);
     } else {
-        abort();
+        g_assert_not_reached();
     }
 
     /* check out of RAM access */
-    if (!address_space_access_valid(&address_space_memory, raddr,
-                                    TARGET_PAGE_SIZE, rw,
+    if (!fail &&
+        !address_space_access_valid(&address_space_memory, raddr,
+                                    TARGET_PAGE_SIZE, access_type,
                                     MEMTXATTRS_UNSPECIFIED)) {
         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);
-        return 1;
+        fail = 1;
     }
 
-    qemu_log_mask(CPU_LOG_MMU, "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
-            __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);
+    if (!fail) {
+        qemu_log_mask(CPU_LOG_MMU,
+                      "%s: set tlb %" PRIx64 " -> %" PRIx64 " (%x)\n",
+                      __func__, (uint64_t)vaddr, (uint64_t)raddr, prot);
+        tlb_set_page(cs, address & TARGET_PAGE_MASK, raddr, prot,
+                     mmu_idx, TARGET_PAGE_SIZE);
+        return true;
+    }
+    if (probe) {
+        return false;
+    }
 
-    tlb_set_page(cs, orig_vaddr & TARGET_PAGE_MASK, raddr, prot,
-                 mmu_idx, TARGET_PAGE_SIZE);
+    cpu_restore_state(cs, retaddr, true);
 
-    return 0;
+    /*
+     * 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.
+     */
+    if (access_type == MMU_INST_FETCH) {
+        env->int_pgm_ilen = 2;
+    }
+
+    cpu_loop_exit(cs);
+}
+
+void tlb_fill(CPUState *cs, target_ulong addr, int size,
+              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+    s390_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
 }
 
 static void do_program_interrupt(CPUS390XState *env)
diff --git a/target/s390x/mem_helper.c b/target/s390x/mem_helper.c
index 3f76a8abfd..ffd5f02fbe 100644
--- a/target/s390x/mem_helper.c
+++ b/target/s390x/mem_helper.c
@@ -33,22 +33,6 @@ 
 
 /*****************************************************************************/
 /* Softmmu support */
-#if !defined(CONFIG_USER_ONLY)
-
-/* try to fill the TLB and return an exception if error. If retaddr is
-   NULL, it means that the function was called in C code (i.e. not
-   from generated code or from helper.c) */
-/* XXX: fix it to restore all registers */
-void tlb_fill(CPUState *cs, target_ulong addr, int size,
-              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
-{
-    int ret = s390_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);
-    if (unlikely(ret != 0)) {
-        cpu_loop_exit_restore(cs, retaddr);
-    }
-}
-
-#endif
 
 /* #define DEBUG_HELPER */
 #ifdef DEBUG_HELPER