[Qemu-devel] [PULL v2 00/27] tcg: Add CPUClass::tlb_fill

[Qemu-devel] [PULL v2 00/27] tcg: Add CPUClass::tlb_fill

[Richard Henderson]

Changes in v2:

  * Fix --disable-tcg compilation for x86 and s390x.
    I adjusted every target/ that used any CONFIG_TCG in cpu.c.
    but then afterward I see that only x86 and s390x have had
    their Makefiles adjusted to make --disable-tcg actually work.

  * Fix Werror for 64-bit on 32-bit.

Only re-posting changed patches.


r~


The following changes since commit efb4f3b62c69383a7308d7b739a3193e7c0ccae8:

  Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-05-10 14:49:36 +0100)

are available in the Git repository at:

  https://github.com/rth7680/qemu.git tags/pull-tcg-20190510

for you to fetch changes up to 4811e9095c0491bc6f5450e5012c9c4796b9e59d:

  tcg: Use tlb_fill probe from tlb_vaddr_to_host (2019-05-10 11:12:50 -0700)

----------------------------------------------------------------
Add CPUClass::tlb_fill.
Improve tlb_vaddr_to_host for use by ARM SVE no-fault loads.

----------------------------------------------------------------
Richard Henderson (27):
      tcg: Add CPUClass::tlb_fill
      target/alpha: Convert to CPUClass::tlb_fill
      target/arm: Convert to CPUClass::tlb_fill
      target/cris: Convert to CPUClass::tlb_fill
      target/hppa: Convert to CPUClass::tlb_fill
      target/i386: Convert to CPUClass::tlb_fill
      target/lm32: Convert to CPUClass::tlb_fill
      target/m68k: Convert to CPUClass::tlb_fill
      target/microblaze: Convert to CPUClass::tlb_fill
      target/mips: Pass a valid error to raise_mmu_exception for user-only
      target/mips: Tidy control flow in mips_cpu_handle_mmu_fault
      target/mips: Convert to CPUClass::tlb_fill
      target/moxie: Convert to CPUClass::tlb_fill
      target/nios2: Convert to CPUClass::tlb_fill
      target/openrisc: Convert to CPUClass::tlb_fill
      target/ppc: Convert to CPUClass::tlb_fill
      target/riscv: Convert to CPUClass::tlb_fill
      target/s390x: Convert to CPUClass::tlb_fill
      target/sh4: Convert to CPUClass::tlb_fill
      target/sparc: Convert to CPUClass::tlb_fill
      target/tilegx: Convert to CPUClass::tlb_fill
      target/tricore: Convert to CPUClass::tlb_fill
      target/unicore32: Convert to CPUClass::tlb_fill
      target/xtensa: Convert to CPUClass::tlb_fill
      tcg: Use CPUClass::tlb_fill in cputlb.c
      tcg: Remove CPUClass::handle_mmu_fault
      tcg: Use tlb_fill probe from tlb_vaddr_to_host

 include/exec/cpu_ldst.h         |  50 +++--------
 include/exec/exec-all.h         |   9 --
 include/qom/cpu.h               |  12 ++-
 target/alpha/cpu.h              |   5 +-
 target/arm/internals.h          |  10 ++-
 target/cris/cpu.h               |   5 +-
 target/hppa/cpu.h               |   8 +-
 target/i386/cpu.h               |   5 +-
 target/lm32/cpu.h               |   5 +-
 target/m68k/cpu.h               |   5 +-
 target/microblaze/cpu.h         |   5 +-
 target/mips/internal.h          |   5 +-
 target/moxie/cpu.h              |   5 +-
 target/nios2/cpu.h              |   5 +-
 target/openrisc/cpu.h           |   5 +-
 target/ppc/cpu.h                |   7 +-
 target/riscv/cpu.h              |   5 +-
 target/s390x/internal.h         |   5 +-
 target/sh4/cpu.h                |   5 +-
 target/sparc/cpu.h              |   5 +-
 target/tricore/cpu.h            |   6 +-
 target/unicore32/cpu.h          |   5 +-
 target/xtensa/cpu.h             |   5 +-
 accel/tcg/cputlb.c              |  88 +++++++++++++++++--
 accel/tcg/user-exec.c           |  36 ++------
 target/alpha/cpu.c              |   5 +-
 target/alpha/helper.c           |  24 +++--
 target/alpha/mem_helper.c       |  16 ----
 target/arm/cpu.c                |  22 +----
 target/arm/helper.c             |  90 +++++++++++--------
 target/arm/op_helper.c          |  29 +-----
 target/arm/sve_helper.c         |   6 +-
 target/cris/cpu.c               |   5 +-
 target/cris/helper.c            |  61 ++++++-------
 target/cris/op_helper.c         |  28 ------
 target/hppa/cpu.c               |   5 +-
 target/hppa/mem_helper.c        |  16 ++--
 target/i386/cpu.c               |   5 +-
 target/i386/excp_helper.c       |  53 ++++++-----
 target/i386/mem_helper.c        |  21 -----
 target/lm32/cpu.c               |   5 +-
 target/lm32/helper.c            |   8 +-
 target/lm32/op_helper.c         |  16 ----
 target/m68k/cpu.c               |   2 +-
 target/m68k/helper.c            |  89 +++++++++----------
 target/m68k/op_helper.c         |  15 ----
 target/microblaze/cpu.c         |   5 +-
 target/microblaze/helper.c      | 101 ++++++++++-----------
 target/microblaze/op_helper.c   |  19 ----
 target/mips/cpu.c               |   5 +-
 target/mips/helper.c            |  81 ++++++++---------
 target/mips/op_helper.c         |  15 ----
 target/moxie/cpu.c              |   5 +-
 target/moxie/helper.c           |  65 +++-----------
 target/nios2/cpu.c              |   5 +-
 target/nios2/helper.c           | 170 +++++++++++++++++-------------------
 target/nios2/mmu.c              |  12 ---
 target/openrisc/cpu.c           |   5 +-
 target/openrisc/mmu.c           |  69 +++++++--------
 target/ppc/mmu_helper.c         |  16 ++--
 target/ppc/translate_init.inc.c |   5 +-
 target/ppc/user_only_helper.c   |  14 +--
 target/riscv/cpu.c              |   5 +-
 target/riscv/cpu_helper.c       |  50 +++++------
 target/s390x/cpu.c              |   5 +-
 target/s390x/excp_helper.c      |  67 +++++++++-----
 target/s390x/mem_helper.c       |  16 ----
 target/sh4/cpu.c                |   5 +-
 target/sh4/helper.c             | 189 +++++++++++++++++++---------------------
 target/sh4/op_helper.c          |  12 ---
 target/sparc/cpu.c              |   5 +-
 target/sparc/ldst_helper.c      |  15 ----
 target/sparc/mmu_helper.c       |  58 +++++++-----
 target/tilegx/cpu.c             |  10 ++-
 target/tricore/cpu.c            |   1 +
 target/tricore/helper.c         |  23 +++--
 target/tricore/op_helper.c      |  26 ------
 target/unicore32/cpu.c          |   5 +-
 target/unicore32/helper.c       |  23 -----
 target/unicore32/op_helper.c    |  14 ---
 target/unicore32/softmmu.c      |  13 ++-
 target/xtensa/cpu.c             |   5 +-
 target/xtensa/helper.c          |  33 ++++---
 83 files changed, 868 insertions(+), 1131 deletions(-)

[Qemu-devel] [PULL v2 03/27] target/arm: Convert to CPUClass::tlb_fill

[Richard Henderson]

Cc: qemu-arm@nongnu.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/internals.h | 10 +++--
 target/arm/cpu.c       | 22 +---------
 target/arm/helper.c    | 98 ++++++++++++++++++++++++++----------------
 target/arm/op_helper.c | 29 ++-----------
 4 files changed, 73 insertions(+), 86 deletions(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index 587a1ddf58..5a02f458f3 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -761,10 +761,12 @@ static inline bool arm_extabort_type(MemTxResult result)
     return result != MEMTX_DECODE_ERROR;
 }
 
-/* Do a page table walk and add page to TLB if possible */
-bool arm_tlb_fill(CPUState *cpu, vaddr address,
-                  MMUAccessType access_type, int mmu_idx,
-                  ARMMMUFaultInfo *fi);
+bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr);
+
+void arm_deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type,
+                       int mmu_idx, ARMMMUFaultInfo *fi) QEMU_NORETURN;
 
 /* Return true if the stage 1 translation regime is using LPAE format page
  * tables */
diff --git a/target/arm/cpu.c b/target/arm/cpu.c
index a181fa8dc1..8eee1d8c59 100644
--- a/target/arm/cpu.c
+++ b/target/arm/cpu.c
@@ -2133,23 +2133,6 @@ static Property arm_cpu_properties[] = {
     DEFINE_PROP_END_OF_LIST()
 };
 
-#ifdef CONFIG_USER_ONLY
-static int arm_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
-                                    int rw, int mmu_idx)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-
-    env->exception.vaddress = address;
-    if (rw == 2) {
-        cs->exception_index = EXCP_PREFETCH_ABORT;
-    } else {
-        cs->exception_index = EXCP_DATA_ABORT;
-    }
-    return 1;
-}
-#endif
-
 static gchar *arm_gdb_arch_name(CPUState *cs)
 {
     ARMCPU *cpu = ARM_CPU(cs);
@@ -2182,9 +2165,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->synchronize_from_tb = arm_cpu_synchronize_from_tb;
     cc->gdb_read_register = arm_cpu_gdb_read_register;
     cc->gdb_write_register = arm_cpu_gdb_write_register;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = arm_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->do_interrupt = arm_cpu_do_interrupt;
     cc->do_unaligned_access = arm_cpu_do_unaligned_access;
     cc->do_transaction_failed = arm_cpu_do_transaction_failed;
@@ -2209,6 +2190,7 @@ static void arm_cpu_class_init(ObjectClass *oc, void *data)
     cc->disas_set_info = arm_disas_set_info;
 #ifdef CONFIG_TCG
     cc->tcg_initialize = arm_translate_init;
+    cc->tlb_fill = arm_cpu_tlb_fill;
 #endif
 }
 
diff --git a/target/arm/helper.c b/target/arm/helper.c
index 1e6eb0d0f3..f1a2b94ddb 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -12596,43 +12596,6 @@ static bool get_phys_addr(CPUARMState *env, target_ulong address,
     }
 }
 
-/* Walk the page table and (if the mapping exists) add the page
- * to the TLB. Return false on success, or true on failure. Populate
- * fsr with ARM DFSR/IFSR fault register format value on failure.
- */
-bool arm_tlb_fill(CPUState *cs, vaddr address,
-                  MMUAccessType access_type, int mmu_idx,
-                  ARMMMUFaultInfo *fi)
-{
-    ARMCPU *cpu = ARM_CPU(cs);
-    CPUARMState *env = &cpu->env;
-    hwaddr phys_addr;
-    target_ulong page_size;
-    int prot;
-    int ret;
-    MemTxAttrs attrs = {};
-
-    ret = get_phys_addr(env, address, access_type,
-                        core_to_arm_mmu_idx(env, mmu_idx), &phys_addr,
-                        &attrs, &prot, &page_size, fi, NULL);
-    if (!ret) {
-        /*
-         * Map a single [sub]page. Regions smaller than our declared
-         * target page size are handled specially, so for those we
-         * pass in the exact addresses.
-         */
-        if (page_size >= TARGET_PAGE_SIZE) {
-            phys_addr &= TARGET_PAGE_MASK;
-            address &= TARGET_PAGE_MASK;
-        }
-        tlb_set_page_with_attrs(cs, address, phys_addr, attrs,
-                                prot, mmu_idx, page_size);
-        return 0;
-    }
-
-    return ret;
-}
-
 hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs, vaddr addr,
                                          MemTxAttrs *attrs)
 {
@@ -13111,6 +13074,67 @@ uint32_t HELPER(v7m_tt)(CPUARMState *env, uint32_t addr, uint32_t op)
 
 #endif
 
+bool arm_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr)
+{
+    ARMCPU *cpu = ARM_CPU(cs);
+
+#ifdef CONFIG_USER_ONLY
+    cpu->env.exception.vaddress = address;
+    if (access_type == MMU_INST_FETCH) {
+        cs->exception_index = EXCP_PREFETCH_ABORT;
+    } else {
+        cs->exception_index = EXCP_DATA_ABORT;
+    }
+    cpu_loop_exit_restore(cs, retaddr);
+#else
+    hwaddr phys_addr;
+    target_ulong page_size;
+    int prot, ret;
+    MemTxAttrs attrs = {};
+    ARMMMUFaultInfo fi = {};
+
+    /*
+     * Walk the page table and (if the mapping exists) add the page
+     * to the TLB.  On success, return true.  Otherwise, if probing,
+     * return false.  Otherwise populate fsr with ARM DFSR/IFSR fault
+     * register format, and signal the fault.
+     */
+    ret = get_phys_addr(&cpu->env, address, access_type,
+                        core_to_arm_mmu_idx(&cpu->env, mmu_idx),
+                        &phys_addr, &attrs, &prot, &page_size, &fi, NULL);
+    if (likely(!ret)) {
+        /*
+         * Map a single [sub]page. Regions smaller than our declared
+         * target page size are handled specially, so for those we
+         * pass in the exact addresses.
+         */
+        if (page_size >= TARGET_PAGE_SIZE) {
+            phys_addr &= TARGET_PAGE_MASK;
+            address &= TARGET_PAGE_MASK;
+        }
+        tlb_set_page_with_attrs(cs, address, phys_addr, attrs,
+                                prot, mmu_idx, page_size);
+        return true;
+    } else if (probe) {
+        return false;
+    } else {
+        /* now we have a real cpu fault */
+        cpu_restore_state(cs, retaddr, true);
+        arm_deliver_fault(cpu, address, access_type, mmu_idx, &fi);
+    }
+#endif
+}
+
+#ifndef CONFIG_USER_ONLY
+void tlb_fill(CPUState *cs, target_ulong addr, int size,
+              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+    arm_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
+}
+#endif
+
 void HELPER(dc_zva)(CPUARMState *env, uint64_t vaddr_in)
 {
     /* Implement DC ZVA, which zeroes a fixed-length block of memory.
diff --git a/target/arm/op_helper.c b/target/arm/op_helper.c
index 8698b4dc83..8ee15a4bd4 100644
--- a/target/arm/op_helper.c
+++ b/target/arm/op_helper.c
@@ -126,8 +126,8 @@ static inline uint32_t merge_syn_data_abort(uint32_t template_syn,
     return syn;
 }
 
-static void deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type,
-                          int mmu_idx, ARMMMUFaultInfo *fi)
+void arm_deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type,
+                       int mmu_idx, ARMMMUFaultInfo *fi)
 {
     CPUARMState *env = &cpu->env;
     int target_el;
@@ -179,27 +179,6 @@ static void deliver_fault(ARMCPU *cpu, vaddr addr, MMUAccessType access_type,
     raise_exception(env, exc, syn, target_el);
 }
 
-/* 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)
- */
-void tlb_fill(CPUState *cs, target_ulong addr, int size,
-              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
-{
-    bool ret;
-    ARMMMUFaultInfo fi = {};
-
-    ret = arm_tlb_fill(cs, addr, access_type, mmu_idx, &fi);
-    if (unlikely(ret)) {
-        ARMCPU *cpu = ARM_CPU(cs);
-
-        /* now we have a real cpu fault */
-        cpu_restore_state(cs, retaddr, true);
-
-        deliver_fault(cpu, addr, access_type, mmu_idx, &fi);
-    }
-}
-
 /* Raise a data fault alignment exception for the specified virtual address */
 void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
                                  MMUAccessType access_type,
@@ -212,7 +191,7 @@ void arm_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
     cpu_restore_state(cs, retaddr, true);
 
     fi.type = ARMFault_Alignment;
-    deliver_fault(cpu, vaddr, access_type, mmu_idx, &fi);
+    arm_deliver_fault(cpu, vaddr, access_type, mmu_idx, &fi);
 }
 
 /* arm_cpu_do_transaction_failed: handle a memory system error response
@@ -233,7 +212,7 @@ void arm_cpu_do_transaction_failed(CPUState *cs, hwaddr physaddr,
 
     fi.ea = arm_extabort_type(response);
     fi.type = ARMFault_SyncExternal;
-    deliver_fault(cpu, addr, access_type, mmu_idx, &fi);
+    arm_deliver_fault(cpu, addr, access_type, mmu_idx, &fi);
 }
 
 #endif /* !defined(CONFIG_USER_ONLY) */
-- 
2.17.1

[Qemu-devel] [PULL v2 06/27] target/i386: Convert to CPUClass::tlb_fill

[Richard Henderson]

We do not support probing, but we do not need it yet either.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Eduardo Habkost <ehabkost@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/i386/cpu.h         |  5 ++--
 target/i386/cpu.c         |  5 ++--
 target/i386/excp_helper.c | 61 +++++++++++++++++++++++++--------------
 target/i386/mem_helper.c  | 21 --------------
 4 files changed, 44 insertions(+), 48 deletions(-)

diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 0128910661..fce6660bac 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -1656,8 +1656,9 @@ void host_cpuid(uint32_t function, uint32_t count,
 void host_vendor_fms(char *vendor, int *family, int *model, int *stepping);
 
 /* helper.c */
-int x86_cpu_handle_mmu_fault(CPUState *cpu, vaddr addr, int size,
-                             int is_write, int mmu_idx);
+bool x86_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr);
 void x86_cpu_set_a20(X86CPU *cpu, int a20_state);
 
 #ifndef CONFIG_USER_ONLY
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 722c5514d4..2df56fa977 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -5915,9 +5915,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->gdb_write_register = x86_cpu_gdb_write_register;
     cc->get_arch_id = x86_cpu_get_arch_id;
     cc->get_paging_enabled = x86_cpu_get_paging_enabled;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = x86_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->asidx_from_attrs = x86_asidx_from_attrs;
     cc->get_memory_mapping = x86_cpu_get_memory_mapping;
     cc->get_phys_page_debug = x86_cpu_get_phys_page_debug;
@@ -5942,6 +5940,7 @@ static void x86_cpu_common_class_init(ObjectClass *oc, void *data)
     cc->cpu_exec_exit = x86_cpu_exec_exit;
 #ifdef CONFIG_TCG
     cc->tcg_initialize = tcg_x86_init;
+    cc->tlb_fill = x86_cpu_tlb_fill;
 #endif
     cc->disas_set_info = x86_disas_set_info;
 
diff --git a/target/i386/excp_helper.c b/target/i386/excp_helper.c
index 49231f6b69..68bf8e3f7c 100644
--- a/target/i386/excp_helper.c
+++ b/target/i386/excp_helper.c
@@ -137,26 +137,7 @@ void raise_exception_ra(CPUX86State *env, int exception_index, uintptr_t retaddr
     raise_interrupt2(env, exception_index, 0, 0, 0, retaddr);
 }
 
-#if defined(CONFIG_USER_ONLY)
-int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr, int size,
-                             int is_write, int mmu_idx)
-{
-    X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
-
-    /* user mode only emulation */
-    is_write &= 1;
-    env->cr[2] = addr;
-    env->error_code = (is_write << PG_ERROR_W_BIT);
-    env->error_code |= PG_ERROR_U_MASK;
-    cs->exception_index = EXCP0E_PAGE;
-    env->exception_is_int = 0;
-    env->exception_next_eip = -1;
-    return 1;
-}
-
-#else
-
+#if !defined(CONFIG_USER_ONLY)
 static hwaddr get_hphys(CPUState *cs, hwaddr gphys, MMUAccessType access_type,
                         int *prot)
 {
@@ -365,8 +346,8 @@ static hwaddr get_hphys(CPUState *cs, hwaddr gphys, MMUAccessType access_type,
  * 0  = nothing more to do
  * 1  = generate PF fault
  */
-int x86_cpu_handle_mmu_fault(CPUState *cs, vaddr addr, int size,
-                             int is_write1, int mmu_idx)
+static int handle_mmu_fault(CPUState *cs, vaddr addr, int size,
+                            int is_write1, int mmu_idx)
 {
     X86CPU *cpu = X86_CPU(cs);
     CPUX86State *env = &cpu->env;
@@ -691,3 +672,39 @@ do_check_protect_pse36:
     return 1;
 }
 #endif
+
+bool x86_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+#ifdef CONFIG_USER_ONLY
+    /* user mode only emulation */
+    env->cr[2] = addr;
+    env->error_code = (access_type == MMU_DATA_STORE) << PG_ERROR_W_BIT;
+    env->error_code |= PG_ERROR_U_MASK;
+    cs->exception_index = EXCP0E_PAGE;
+    env->exception_is_int = 0;
+    env->exception_next_eip = -1;
+    cpu_loop_exit_restore(cs, retaddr);
+#else
+    env->retaddr = retaddr;
+    if (handle_mmu_fault(cs, addr, size, access_type, mmu_idx)) {
+        /* FIXME: On error in get_hphys we have already jumped out.  */
+        g_assert(!probe);
+        raise_exception_err_ra(env, cs->exception_index,
+                               env->error_code, retaddr);
+    }
+    return true;
+#endif
+}
+
+#if !defined(CONFIG_USER_ONLY)
+void tlb_fill(CPUState *cs, target_ulong addr, int size,
+              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+    x86_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
+}
+#endif
diff --git a/target/i386/mem_helper.c b/target/i386/mem_helper.c
index 6cc53bcb40..1885df29d2 100644
--- a/target/i386/mem_helper.c
+++ b/target/i386/mem_helper.c
@@ -191,24 +191,3 @@ void helper_boundl(CPUX86State *env, target_ulong a0, int v)
         raise_exception_ra(env, EXCP05_BOUND, GETPC());
     }
 }
-
-#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)
-{
-    X86CPU *cpu = X86_CPU(cs);
-    CPUX86State *env = &cpu->env;
-    int ret;
-
-    env->retaddr = retaddr;
-    ret = x86_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);
-    if (ret) {
-        raise_exception_err_ra(env, cs->exception_index, env->error_code, retaddr);
-    }
-}
-#endif
-- 
2.17.1

[Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Richard Henderson]

Note that env->active_tc.PC is removed from the qemu_log as that value
is garbage.  The PC isn't recovered until cpu_restore_state, called from
cpu_loop_exit_restore, called from do_raise_exception_err.

Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/mips/internal.h  |  5 +++--
 target/mips/cpu.c       |  5 ++---
 target/mips/helper.c    | 45 ++++++++++++++++++++++-------------------
 target/mips/op_helper.c | 15 --------------
 4 files changed, 29 insertions(+), 41 deletions(-)

diff --git a/target/mips/internal.h b/target/mips/internal.h
index 286e3888ab..b2b41a51ab 100644
--- a/target/mips/internal.h
+++ b/target/mips/internal.h
@@ -202,8 +202,9 @@ void cpu_mips_start_count(CPUMIPSState *env);
 void cpu_mips_stop_count(CPUMIPSState *env);
 
 /* helper.c */
-int mips_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw,
-                              int mmu_idx);
+bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool probe, uintptr_t retaddr);
 
 /* op_helper.c */
 uint32_t float_class_s(uint32_t arg, float_status *fst);
diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index e217fb3e36..a33058609a 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -197,9 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
     cc->synchronize_from_tb = mips_cpu_synchronize_from_tb;
     cc->gdb_read_register = mips_cpu_gdb_read_register;
     cc->gdb_write_register = mips_cpu_gdb_write_register;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = mips_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->do_unassigned_access = mips_cpu_unassigned_access;
     cc->do_unaligned_access = mips_cpu_do_unaligned_access;
     cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
@@ -208,6 +206,7 @@ static void mips_cpu_class_init(ObjectClass *c, void *data)
     cc->disas_set_info = mips_cpu_disas_set_info;
 #ifdef CONFIG_TCG
     cc->tcg_initialize = mips_tcg_init;
+    cc->tlb_fill = mips_cpu_tlb_fill;
 #endif
 
     cc->gdb_num_core_regs = 73;
diff --git a/target/mips/helper.c b/target/mips/helper.c
index 86e622efb8..3a4917ce7b 100644
--- a/target/mips/helper.c
+++ b/target/mips/helper.c
@@ -874,31 +874,25 @@ refill:
 #endif
 #endif
 
-int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
-                              int mmu_idx)
+bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                       MMUAccessType access_type, int mmu_idx,
+                       bool probe, uintptr_t retaddr)
 {
     MIPSCPU *cpu = MIPS_CPU(cs);
     CPUMIPSState *env = &cpu->env;
 #if !defined(CONFIG_USER_ONLY)
     hwaddr physical;
     int prot;
-    int access_type;
+    int mips_access_type;
 #endif
     int ret = TLBRET_BADADDR;
 
-#if 0
-    log_cpu_state(cs, 0);
-#endif
-    qemu_log_mask(CPU_LOG_MMU,
-              "%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
-              __func__, env->active_tc.PC, address, rw, mmu_idx);
-
     /* data access */
 #if !defined(CONFIG_USER_ONLY)
     /* XXX: put correct access by using cpu_restore_state() correctly */
-    access_type = ACCESS_INT;
-    ret = get_physical_address(env, &physical, &prot,
-                               address, rw, access_type, mmu_idx);
+    mips_access_type = ACCESS_INT;
+    ret = get_physical_address(env, &physical, &prot, address,
+                               access_type, mips_access_type, mmu_idx);
     switch (ret) {
     case TLBRET_MATCH:
         qemu_log_mask(CPU_LOG_MMU,
@@ -915,7 +909,7 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
         tlb_set_page(cs, address & TARGET_PAGE_MASK,
                      physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
                      mmu_idx, TARGET_PAGE_SIZE);
-        return 0;
+        return true;
     }
 #if !defined(TARGET_MIPS64)
     if ((ret == TLBRET_NOMATCH) && (env->tlb->nb_tlb > 1)) {
@@ -926,27 +920,36 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
         int mode = (env->hflags & MIPS_HFLAG_KSU);
         bool ret_walker;
         env->hflags &= ~MIPS_HFLAG_KSU;
-        ret_walker = page_table_walk_refill(env, address, rw, mmu_idx);
+        ret_walker = page_table_walk_refill(env, address, access_type, mmu_idx);
         env->hflags |= mode;
         if (ret_walker) {
-            ret = get_physical_address(env, &physical, &prot,
-                                       address, rw, access_type, mmu_idx);
+            ret = get_physical_address(env, &physical, &prot, address,
+                                       access_type, mips_access_type, mmu_idx);
             if (ret == TLBRET_MATCH) {
                 tlb_set_page(cs, address & TARGET_PAGE_MASK,
                              physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
                              mmu_idx, TARGET_PAGE_SIZE);
-                return 0;
+                return true;
             }
         }
     }
 #endif
+    if (probe) {
+        return false;
+    }
 #endif
 
-    raise_mmu_exception(env, address, rw, ret);
-    return 1;
+    raise_mmu_exception(env, address, access_type, ret);
+    do_raise_exception_err(env, cs->exception_index, env->error_code, retaddr);
+}
+
+#ifndef CONFIG_USER_ONLY
+void tlb_fill(CPUState *cs, target_ulong addr, int size,
+              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+    mips_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
 }
 
-#if !defined(CONFIG_USER_ONLY)
 hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong address, int rw)
 {
     hwaddr physical;
diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
index 0f272a5b93..6d86912958 100644
--- a/target/mips/op_helper.c
+++ b/target/mips/op_helper.c
@@ -2669,21 +2669,6 @@ void mips_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     do_raise_exception_err(env, excp, error_code, retaddr);
 }
 
-void tlb_fill(CPUState *cs, target_ulong addr, int size,
-              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
-{
-    int ret;
-
-    ret = mips_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);
-    if (ret) {
-        MIPSCPU *cpu = MIPS_CPU(cs);
-        CPUMIPSState *env = &cpu->env;
-
-        do_raise_exception_err(env, cs->exception_index,
-                               env->error_code, retaddr);
-    }
-}
-
 void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
                                 bool is_write, bool is_exec, int unused,
                                 unsigned size)
-- 
2.17.1

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Aleksandar Markovic]

On May 10, 2019 8:57 PM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>

Please change the title to 'target/mips: Switch to using
mips_cpu_tlb_fill()', or something along that line.

Also, the reason for changing the field access_type to mips_access type
should be explained in the commit message.

This commit message is generally poor, as it explains relatively
unimportant logging issue, while not explaining the core of the change.

Thanks,
Aleksandar

> Note that env->active_tc.PC is removed from the qemu_log as that value
> is garbage.  The PC isn't recovered until cpu_restore_state, called from
> cpu_loop_exit_restore, called from do_raise_exception_err.
>
> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
> Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/mips/internal.h  |  5 +++--
>  target/mips/cpu.c       |  5 ++---
>  target/mips/helper.c    | 45 ++++++++++++++++++++++-------------------
>  target/mips/op_helper.c | 15 --------------
>  4 files changed, 29 insertions(+), 41 deletions(-)
>
> diff --git a/target/mips/internal.h b/target/mips/internal.h
> index 286e3888ab..b2b41a51ab 100644
> --- a/target/mips/internal.h
> +++ b/target/mips/internal.h
> @@ -202,8 +202,9 @@ void cpu_mips_start_count(CPUMIPSState *env);
>  void cpu_mips_stop_count(CPUMIPSState *env);
>
>  /* helper.c */
> -int mips_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size,
int rw,
> -                              int mmu_idx);
> +bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> +                       MMUAccessType access_type, int mmu_idx,
> +                       bool probe, uintptr_t retaddr);
>
>  /* op_helper.c */
>  uint32_t float_class_s(uint32_t arg, float_status *fst);
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index e217fb3e36..a33058609a 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -197,9 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void
*data)
>      cc->synchronize_from_tb = mips_cpu_synchronize_from_tb;
>      cc->gdb_read_register = mips_cpu_gdb_read_register;
>      cc->gdb_write_register = mips_cpu_gdb_write_register;
> -#ifdef CONFIG_USER_ONLY
> -    cc->handle_mmu_fault = mips_cpu_handle_mmu_fault;
> -#else
> +#ifndef CONFIG_USER_ONLY
>      cc->do_unassigned_access = mips_cpu_unassigned_access;
>      cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>      cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
> @@ -208,6 +206,7 @@ static void mips_cpu_class_init(ObjectClass *c, void
*data)
>      cc->disas_set_info = mips_cpu_disas_set_info;
>  #ifdef CONFIG_TCG
>      cc->tcg_initialize = mips_tcg_init;
> +    cc->tlb_fill = mips_cpu_tlb_fill;
>  #endif
>
>      cc->gdb_num_core_regs = 73;
> diff --git a/target/mips/helper.c b/target/mips/helper.c
> index 86e622efb8..3a4917ce7b 100644
> --- a/target/mips/helper.c
> +++ b/target/mips/helper.c
> @@ -874,31 +874,25 @@ refill:
>  #endif
>  #endif
>
> -int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int
rw,
> -                              int mmu_idx)
> +bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
> +                       MMUAccessType access_type, int mmu_idx,
> +                       bool probe, uintptr_t retaddr)
>  {
>      MIPSCPU *cpu = MIPS_CPU(cs);
>      CPUMIPSState *env = &cpu->env;
>  #if !defined(CONFIG_USER_ONLY)
>      hwaddr physical;
>      int prot;
> -    int access_type;
> +    int mips_access_type;
>  #endif
>      int ret = TLBRET_BADADDR;
>
> -#if 0
> -    log_cpu_state(cs, 0);
> -#endif
> -    qemu_log_mask(CPU_LOG_MMU,
> -              "%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx
%d\n",
> -              __func__, env->active_tc.PC, address, rw, mmu_idx);
> -
>      /* data access */
>  #if !defined(CONFIG_USER_ONLY)
>      /* XXX: put correct access by using cpu_restore_state() correctly */
> -    access_type = ACCESS_INT;
> -    ret = get_physical_address(env, &physical, &prot,
> -                               address, rw, access_type, mmu_idx);
> +    mips_access_type = ACCESS_INT;
> +    ret = get_physical_address(env, &physical, &prot, address,
> +                               access_type, mips_access_type, mmu_idx);
>      switch (ret) {
>      case TLBRET_MATCH:
>          qemu_log_mask(CPU_LOG_MMU,
> @@ -915,7 +909,7 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr
address, int size, int rw,
>          tlb_set_page(cs, address & TARGET_PAGE_MASK,
>                       physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
>                       mmu_idx, TARGET_PAGE_SIZE);
> -        return 0;
> +        return true;
>      }
>  #if !defined(TARGET_MIPS64)
>      if ((ret == TLBRET_NOMATCH) && (env->tlb->nb_tlb > 1)) {
> @@ -926,27 +920,36 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr
address, int size, int rw,
>          int mode = (env->hflags & MIPS_HFLAG_KSU);
>          bool ret_walker;
>          env->hflags &= ~MIPS_HFLAG_KSU;
> -        ret_walker = page_table_walk_refill(env, address, rw, mmu_idx);
> +        ret_walker = page_table_walk_refill(env, address, access_type,
mmu_idx);
>          env->hflags |= mode;
>          if (ret_walker) {
> -            ret = get_physical_address(env, &physical, &prot,
> -                                       address, rw, access_type,
mmu_idx);
> +            ret = get_physical_address(env, &physical, &prot, address,
> +                                       access_type, mips_access_type,
mmu_idx);¿
>              if (ret == TLBRET_MATCH) {
>                  tlb_set_page(cs, address & TARGET_PAGE_MASK,
>                               physical & TARGET_PAGE_MASK, prot |
PAGE_EXEC,
>                               mmu_idx, TARGET_PAGE_SIZE);
> -                return 0;
> +                return true;
>              }
>          }
>      }
>  #endif
> +    if (probe) {
> +        return false;
> +    }
>  #endif
>
> -    raise_mmu_exception(env, address, rw, ret);
> -    return 1;
> +    raise_mmu_exception(env, address, access_type, ret);
> +    do_raise_exception_err(env, cs->exception_index, env->error_code,
retaddr);
> +}
> +
> +#ifndef CONFIG_USER_ONLY
> +void tlb_fill(CPUState *cs, target_ulong addr, int size,
> +              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> +{
> +    mips_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false,
retaddr);
>  }
>
> -#if !defined(CONFIG_USER_ONLY)
>  hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong
address, int rw)
>  {
>      hwaddr physical;
> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
> index 0f272a5b93..6d86912958 100644
> --- a/target/mips/op_helper.c
> +++ b/target/mips/op_helper.c
> @@ -2669,21 +2669,6 @@ void mips_cpu_do_unaligned_access(CPUState *cs,
vaddr addr,
>      do_raise_exception_err(env, excp, error_code, retaddr);
>  }
>
> -void tlb_fill(CPUState *cs, target_ulong addr, int size,
> -              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
> -{
> -    int ret;
> -
> -    ret = mips_cpu_handle_mmu_fault(cs, addr, size, access_type,
mmu_idx);
> -    if (ret) {
> -        MIPSCPU *cpu = MIPS_CPU(cs);
> -        CPUMIPSState *env = &cpu->env;
> -
> -        do_raise_exception_err(env, cs->exception_index,
> -                               env->error_code, retaddr);
> -    }
> -}
> -
>  void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>                                  bool is_write, bool is_exec, int unused,
>                                  unsigned size)
> --
> 2.17.1
>
>

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Peter Maydell]

On Sat, 11 May 2019 at 14:43, Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
> This commit message is generally poor, as it explains relatively unimportant logging issue, while not explaining the core of the change.

I think the assumption with this sort of "refactor to change
all instances of an API" change is that you will go and
look at the commit message (or cover letter for the patch
series) that introduces the new API; there's no great need
to repeat the same justification for every commit that
applies the refactoring to each of our dozen or more
guest front-ends.

I'm not sure that wordsmithing a commit message really
justifies rerolling this pull request at this point.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Aleksandar Markovic]

On May 14, 2019 5:26 PM, "Peter Maydell" <peter.maydell@linaro.org> wrote:
>
> On Sat, 11 May 2019 at 14:43, Aleksandar Markovic
> <aleksandar.m.mail@gmail.com> wrote:
> > This commit message is generally poor, as it explains relatively
unimportant logging issue, while not explaining the core of the change.
>
> I think the assumption with this sort of "refactor to change
> all instances of an API" change is that you will go and
> look at the commit message (or cover letter for the patch
> series) that introduces the new API; there's no great need
> to repeat the same justification for every commit that
> applies the refactoring to each of our dozen or more
> guest front-ends.
>
> I'm not sure that wordsmithing a commit message really
> justifies rerolling this pull request at this point.
>
> thanks
> -- PMM

Peter, hi.

Thanks for your engaging in the discussion!

I am not complaining about myself looking at another commit message, but
have future maintainers and future developers in mind. Their effort needed
for deciphering commit messages like this one is multiple times larger than
putting together a clear, full, and right-on-the-money message by the
submitter. The commit messages should be made convenient for their readers,
not writters, shouldn't they?

That being said, I tend to accept your judgement, and I withdraw any
request for changing elements of this pull request. Please go ahead with
integrating it, and thanks again for involving in this issue, and sorry for
taking your time.

Sincerely,
Aleksandar

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Peter Maydell]

On Tue, 14 May 2019 at 17:05, Aleksandar Markovic
<aleksandar.m.mail@gmail.com> wrote:
> I am not complaining about myself looking at another commit message, but have future maintainers and future developers in mind. Their effort needed for deciphering commit messages like this one is multiple times larger than putting together a clear, full, and right-on-the-money message by the submitter. The commit messages should be made convenient for their readers, not writters, shouldn't they?

Yeah, good commit messages are important; in the end there
is a judgement call to be made about how much detail is useful.

I think one thing that might have affected our differing
views here is that you were only cc'd on the MIPS-related patch,
so will have been looking at it in isolation. I read the whole
series starting with the initial patches which introduced the
API, so had a fuller context for looking at this patch.
(I think future readers will also be able to acquire that
fuller context easily because they can just look through
the git history for the nearby commit that adds the tlb_fill
method if they want the rationale for the refactoring.)

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Alex Bennée]

Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> On May 10, 2019 8:57 PM, "Richard Henderson" <richard.henderson@linaro.org>
> wrote:
>>
>
> Please change the title to 'target/mips: Switch to using
> mips_cpu_tlb_fill()', or something along that line.

It does seem a little redundant as "target/mips:" already marks it as a
mips specific change and viewing the log you can see a series of
architectures being converted to a new API.

> Also, the reason for changing the field access_type to mips_access type
> should be explained in the commit message.

ok

> This commit message is generally poor, as it explains relatively
> unimportant logging issue, while not explaining the core of the
> change.

Surely the core of the change is explained in the main patches that
introduce the new API? I think it would be redundant to repeat that for
every individual architecture touched. It's a shame it's hard to
explicitly reference a patch in the same series as the commit hashes are
not yet permanent. At least when we fix things referring to the short
hash of the original commit is fairly easy.

Generally for an architecture conversion I want to know what might be
different from other architectures converted. If it is a broadly
mechanical change it doesn't need to be too detailed.

>
> Thanks,
> Aleksandar
>
>> Note that env->active_tc.PC is removed from the qemu_log as that value
>> is garbage.  The PC isn't recovered until cpu_restore_state, called from
>> cpu_loop_exit_restore, called from do_raise_exception_err.
>>
>> Cc: Aleksandar Markovic <amarkovic@wavecomp.com>
>> Cc: Aleksandar Rikalo <arikalo@wavecomp.com>
>> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  target/mips/internal.h  |  5 +++--
>>  target/mips/cpu.c       |  5 ++---
>>  target/mips/helper.c    | 45 ++++++++++++++++++++++-------------------
>>  target/mips/op_helper.c | 15 --------------
>>  4 files changed, 29 insertions(+), 41 deletions(-)
>>
>> diff --git a/target/mips/internal.h b/target/mips/internal.h
>> index 286e3888ab..b2b41a51ab 100644
>> --- a/target/mips/internal.h
>> +++ b/target/mips/internal.h
>> @@ -202,8 +202,9 @@ void cpu_mips_start_count(CPUMIPSState *env);
>>  void cpu_mips_stop_count(CPUMIPSState *env);
>>
>>  /* helper.c */
>> -int mips_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size,
> int rw,
>> -                              int mmu_idx);
>> +bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>> +                       MMUAccessType access_type, int mmu_idx,
>> +                       bool probe, uintptr_t retaddr);
>>
>>  /* op_helper.c */
>>  uint32_t float_class_s(uint32_t arg, float_status *fst);
>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>> index e217fb3e36..a33058609a 100644
>> --- a/target/mips/cpu.c
>> +++ b/target/mips/cpu.c
>> @@ -197,9 +197,7 @@ static void mips_cpu_class_init(ObjectClass *c, void
> *data)
>>      cc->synchronize_from_tb = mips_cpu_synchronize_from_tb;
>>      cc->gdb_read_register = mips_cpu_gdb_read_register;
>>      cc->gdb_write_register = mips_cpu_gdb_write_register;
>> -#ifdef CONFIG_USER_ONLY
>> -    cc->handle_mmu_fault = mips_cpu_handle_mmu_fault;
>> -#else
>> +#ifndef CONFIG_USER_ONLY
>>      cc->do_unassigned_access = mips_cpu_unassigned_access;
>>      cc->do_unaligned_access = mips_cpu_do_unaligned_access;
>>      cc->get_phys_page_debug = mips_cpu_get_phys_page_debug;
>> @@ -208,6 +206,7 @@ static void mips_cpu_class_init(ObjectClass *c, void
> *data)
>>      cc->disas_set_info = mips_cpu_disas_set_info;
>>  #ifdef CONFIG_TCG
>>      cc->tcg_initialize = mips_tcg_init;
>> +    cc->tlb_fill = mips_cpu_tlb_fill;
>>  #endif
>>
>>      cc->gdb_num_core_regs = 73;
>> diff --git a/target/mips/helper.c b/target/mips/helper.c
>> index 86e622efb8..3a4917ce7b 100644
>> --- a/target/mips/helper.c
>> +++ b/target/mips/helper.c
>> @@ -874,31 +874,25 @@ refill:
>>  #endif
>>  #endif
>>
>> -int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int
> rw,
>> -                              int mmu_idx)
>> +bool mips_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
>> +                       MMUAccessType access_type, int mmu_idx,
>> +                       bool probe, uintptr_t retaddr)
>>  {
>>      MIPSCPU *cpu = MIPS_CPU(cs);
>>      CPUMIPSState *env = &cpu->env;
>>  #if !defined(CONFIG_USER_ONLY)
>>      hwaddr physical;
>>      int prot;
>> -    int access_type;
>> +    int mips_access_type;
>>  #endif
>>      int ret = TLBRET_BADADDR;
>>
>> -#if 0
>> -    log_cpu_state(cs, 0);
>> -#endif
>> -    qemu_log_mask(CPU_LOG_MMU,
>> -              "%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx
> %d\n",
>> -              __func__, env->active_tc.PC, address, rw, mmu_idx);
>> -
>>      /* data access */
>>  #if !defined(CONFIG_USER_ONLY)
>>      /* XXX: put correct access by using cpu_restore_state() correctly */
>> -    access_type = ACCESS_INT;
>> -    ret = get_physical_address(env, &physical, &prot,
>> -                               address, rw, access_type, mmu_idx);
>> +    mips_access_type = ACCESS_INT;
>> +    ret = get_physical_address(env, &physical, &prot, address,
>> +                               access_type, mips_access_type, mmu_idx);
>>      switch (ret) {
>>      case TLBRET_MATCH:
>>          qemu_log_mask(CPU_LOG_MMU,
>> @@ -915,7 +909,7 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr
> address, int size, int rw,
>>          tlb_set_page(cs, address & TARGET_PAGE_MASK,
>>                       physical & TARGET_PAGE_MASK, prot | PAGE_EXEC,
>>                       mmu_idx, TARGET_PAGE_SIZE);
>> -        return 0;
>> +        return true;
>>      }
>>  #if !defined(TARGET_MIPS64)
>>      if ((ret == TLBRET_NOMATCH) && (env->tlb->nb_tlb > 1)) {
>> @@ -926,27 +920,36 @@ int mips_cpu_handle_mmu_fault(CPUState *cs, vaddr
> address, int size, int rw,
>>          int mode = (env->hflags & MIPS_HFLAG_KSU);
>>          bool ret_walker;
>>          env->hflags &= ~MIPS_HFLAG_KSU;
>> -        ret_walker = page_table_walk_refill(env, address, rw, mmu_idx);
>> +        ret_walker = page_table_walk_refill(env, address, access_type,
> mmu_idx);
>>          env->hflags |= mode;
>>          if (ret_walker) {
>> -            ret = get_physical_address(env, &physical, &prot,
>> -                                       address, rw, access_type,
> mmu_idx);
>> +            ret = get_physical_address(env, &physical, &prot, address,
>> +                                       access_type, mips_access_type,
> mmu_idx);¿
>>              if (ret == TLBRET_MATCH) {
>>                  tlb_set_page(cs, address & TARGET_PAGE_MASK,
>>                               physical & TARGET_PAGE_MASK, prot |
> PAGE_EXEC,
>>                               mmu_idx, TARGET_PAGE_SIZE);
>> -                return 0;
>> +                return true;
>>              }
>>          }
>>      }
>>  #endif
>> +    if (probe) {
>> +        return false;
>> +    }
>>  #endif
>>
>> -    raise_mmu_exception(env, address, rw, ret);
>> -    return 1;
>> +    raise_mmu_exception(env, address, access_type, ret);
>> +    do_raise_exception_err(env, cs->exception_index, env->error_code,
> retaddr);
>> +}
>> +
>> +#ifndef CONFIG_USER_ONLY
>> +void tlb_fill(CPUState *cs, target_ulong addr, int size,
>> +              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>> +{
>> +    mips_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false,
> retaddr);
>>  }
>>
>> -#if !defined(CONFIG_USER_ONLY)
>>  hwaddr cpu_mips_translate_address(CPUMIPSState *env, target_ulong
> address, int rw)
>>  {
>>      hwaddr physical;
>> diff --git a/target/mips/op_helper.c b/target/mips/op_helper.c
>> index 0f272a5b93..6d86912958 100644
>> --- a/target/mips/op_helper.c
>> +++ b/target/mips/op_helper.c
>> @@ -2669,21 +2669,6 @@ void mips_cpu_do_unaligned_access(CPUState *cs,
> vaddr addr,
>>      do_raise_exception_err(env, excp, error_code, retaddr);
>>  }
>>
>> -void tlb_fill(CPUState *cs, target_ulong addr, int size,
>> -              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
>> -{
>> -    int ret;
>> -
>> -    ret = mips_cpu_handle_mmu_fault(cs, addr, size, access_type,
> mmu_idx);
>> -    if (ret) {
>> -        MIPSCPU *cpu = MIPS_CPU(cs);
>> -        CPUMIPSState *env = &cpu->env;
>> -
>> -        do_raise_exception_err(env, cs->exception_index,
>> -                               env->error_code, retaddr);
>> -    }
>> -}
>> -
>>  void mips_cpu_unassigned_access(CPUState *cs, hwaddr addr,
>>                                  bool is_write, bool is_exec, int unused,
>>                                  unsigned size)
>> --
>> 2.17.1
>>
>>


--
Alex Bennée

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Philippe Mathieu-Daudé]

On 5/14/19 5:48 PM, Alex Bennée wrote:
> 
> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
> 
>> On May 10, 2019 8:57 PM, "Richard Henderson" <richard.henderson@linaro.org>
>> wrote:
>>>
>>
>> Please change the title to 'target/mips: Switch to using
>> mips_cpu_tlb_fill()', or something along that line.
> 
> It does seem a little redundant as "target/mips:" already marks it as a
> mips specific change and viewing the log you can see a series of
> architectures being converted to a new API.
> 
>> Also, the reason for changing the field access_type to mips_access type
>> should be explained in the commit message.
> 
> ok
> 
>> This commit message is generally poor, as it explains relatively
>> unimportant logging issue, while not explaining the core of the
>> change.
> 
> Surely the core of the change is explained in the main patches that
> introduce the new API? I think it would be redundant to repeat that for
> every individual architecture touched. It's a shame it's hard to
> explicitly reference a patch in the same series as the commit hashes are
> not yet permanent. At least when we fix things referring to the short
> hash of the original commit is fairly easy.

Except in the case the maintainer is sending a pull request (like here)
where he can manually fix the commits. Still this is a PITA...

> 
> Generally for an architecture conversion I want to know what might be
> different from other architectures converted. If it is a broadly
> mechanical change it doesn't need to be too detailed.

Re: [Qemu-devel] [PULL v2 12/27] target/mips: Convert to CPUClass::tlb_fill

[Alex Bennée]

Philippe Mathieu-Daudé <philmd@redhat.com> writes:

> On 5/14/19 5:48 PM, Alex Bennée wrote:
>>
>> Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:
>>
>>> On May 10, 2019 8:57 PM, "Richard Henderson" <richard.henderson@linaro.org>
>>> wrote:
>>>>
>>>
>>> Please change the title to 'target/mips: Switch to using
>>> mips_cpu_tlb_fill()', or something along that line.
>>
>> It does seem a little redundant as "target/mips:" already marks it as a
>> mips specific change and viewing the log you can see a series of
>> architectures being converted to a new API.
>>
>>> Also, the reason for changing the field access_type to mips_access type
>>> should be explained in the commit message.
>>
>> ok
>>
>>> This commit message is generally poor, as it explains relatively
>>> unimportant logging issue, while not explaining the core of the
>>> change.
>>
>> Surely the core of the change is explained in the main patches that
>> introduce the new API? I think it would be redundant to repeat that for
>> every individual architecture touched. It's a shame it's hard to
>> explicitly reference a patch in the same series as the commit hashes are
>> not yet permanent. At least when we fix things referring to the short
>> hash of the original commit is fairly easy.
>
> Except in the case the maintainer is sending a pull request (like here)
> where he can manually fix the commits. Still this is a PITA...

If there was tooling that can go from a patch series to a pull request
then maybe. But generally a PR is a series of patches that have now
passed some standard and can now be sent. I'm not sure I'd want to go
over all my commits and re-edit the messages at that point.

--
Alex Bennée

[Qemu-devel] [PULL v2 16/27] target/ppc: Convert to CPUClass::tlb_fill

[Richard Henderson]

Cc: qemu-ppc@nongnu.org
Acked-by: David Gibson <david@gibson.dropbear.id.au>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/cpu.h                |  7 +++----
 target/ppc/mmu_helper.c         | 22 +++++++++++++---------
 target/ppc/translate_init.inc.c |  5 ++---
 target/ppc/user_only_helper.c   | 14 ++++++++------
 4 files changed, 26 insertions(+), 22 deletions(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5e7cf54b2f..d7f23ad5e0 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -1311,10 +1311,9 @@ void ppc_translate_init(void);
  * is returned if the signal was handled by the virtual CPU.
  */
 int cpu_ppc_signal_handler(int host_signum, void *pinfo, void *puc);
-#if defined(CONFIG_USER_ONLY)
-int ppc_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size, int rw,
-                             int mmu_idx);
-#endif
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr);
 
 #if !defined(CONFIG_USER_ONLY)
 void ppc_store_sdr1(CPUPPCState *env, target_ulong value);
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1dbc9acb75..afcca50530 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -3057,15 +3057,9 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
 
 /*****************************************************************************/
 
-/*
- * 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)
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
@@ -3078,7 +3072,17 @@ void tlb_fill(CPUState *cs, target_ulong addr, int size,
         ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
     }
     if (unlikely(ret != 0)) {
+        if (probe) {
+            return false;
+        }
         raise_exception_err_ra(env, cs->exception_index, env->error_code,
                                retaddr);
     }
+    return true;
+}
+
+void tlb_fill(CPUState *cs, target_ulong addr, int size,
+              MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
+{
+    ppc_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
 }
diff --git a/target/ppc/translate_init.inc.c b/target/ppc/translate_init.inc.c
index 0394a9ddad..ad5e14b16f 100644
--- a/target/ppc/translate_init.inc.c
+++ b/target/ppc/translate_init.inc.c
@@ -10592,9 +10592,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
     cc->gdb_read_register = ppc_cpu_gdb_read_register;
     cc->gdb_write_register = ppc_cpu_gdb_write_register;
     cc->do_unaligned_access = ppc_cpu_do_unaligned_access;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = ppc_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->get_phys_page_debug = ppc_cpu_get_phys_page_debug;
     cc->vmsd = &vmstate_ppc_cpu;
 #endif
@@ -10624,6 +10622,7 @@ static void ppc_cpu_class_init(ObjectClass *oc, void *data)
 #endif
 #ifdef CONFIG_TCG
     cc->tcg_initialize = ppc_translate_init;
+    cc->tlb_fill = ppc_cpu_tlb_fill;
 #endif
     cc->disas_set_info = ppc_disas_set_info;
 
diff --git a/target/ppc/user_only_helper.c b/target/ppc/user_only_helper.c
index 2f1477f102..683c03390d 100644
--- a/target/ppc/user_only_helper.c
+++ b/target/ppc/user_only_helper.c
@@ -20,21 +20,24 @@
 
 #include "qemu/osdep.h"
 #include "cpu.h"
+#include "exec/exec-all.h"
 
-int ppc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
-                             int mmu_idx)
+
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                      MMUAccessType access_type, int mmu_idx,
+                      bool probe, uintptr_t retaddr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
     CPUPPCState *env = &cpu->env;
     int exception, error_code;
 
-    if (rw == 2) {
+    if (access_type == MMU_INST_FETCH) {
         exception = POWERPC_EXCP_ISI;
         error_code = 0x40000000;
     } else {
         exception = POWERPC_EXCP_DSI;
         error_code = 0x40000000;
-        if (rw) {
+        if (access_type == MMU_DATA_STORE) {
             error_code |= 0x02000000;
         }
         env->spr[SPR_DAR] = address;
@@ -42,6 +45,5 @@ int ppc_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size, int rw,
     }
     cs->exception_index = exception;
     env->error_code = error_code;
-
-    return 1;
+    cpu_loop_exit_restore(cs, retaddr);
 }
-- 
2.17.1

[Qemu-devel] [PULL v2 17/27] target/riscv: Convert to CPUClass::tlb_fill

[Richard Henderson]

Note that env->pc is removed from the qemu_log as that value is garbage.
The PC isn't recovered until cpu_restore_state, called from
cpu_loop_exit_restore, called from riscv_raise_exception.

Cc: qemu-riscv@nongnu.org
Cc: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h        |  5 +++--
 target/riscv/cpu.c        |  5 ++---
 target/riscv/cpu_helper.c | 46 ++++++++++++++++++---------------------
 3 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7d9f48973f..c17184f4e4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -261,8 +261,9 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                     MMUAccessType access_type, int mmu_idx,
                                     uintptr_t retaddr);
-int riscv_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size,
-                              int rw, int mmu_idx);
+bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                        MMUAccessType access_type, int mmu_idx,
+                        bool probe, uintptr_t retaddr);
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1bcf4eaeb8..b7675707e0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -355,14 +355,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
 #endif
     cc->gdb_stop_before_watchpoint = true;
     cc->disas_set_info = riscv_cpu_disas_set_info;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = riscv_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->do_unaligned_access = riscv_cpu_do_unaligned_access;
     cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug;
 #endif
 #ifdef CONFIG_TCG
     cc->tcg_initialize = riscv_translate_init;
+    cc->tlb_fill = riscv_cpu_tlb_fill;
 #endif
     /* For now, mark unmigratable: */
     cc->vmsd = &vmstate_riscv_cpu;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b17f169681..2535435260 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -379,53 +379,49 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     riscv_raise_exception(env, cs->exception_index, retaddr);
 }
 
-/* called by qemu's softmmu to fill the qemu tlb */
 void tlb_fill(CPUState *cs, target_ulong addr, int size,
         MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
 {
-    int ret;
-    ret = riscv_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);
-    if (ret == TRANSLATE_FAIL) {
-        RISCVCPU *cpu = RISCV_CPU(cs);
-        CPURISCVState *env = &cpu->env;
-        riscv_raise_exception(env, cs->exception_index, retaddr);
-    }
+    riscv_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
 }
-
 #endif
 
-int riscv_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
-        int rw, int mmu_idx)
+bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                        MMUAccessType access_type, int mmu_idx,
+                        bool probe, uintptr_t retaddr)
 {
+#ifndef CONFIG_USER_ONLY
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-#if !defined(CONFIG_USER_ONLY)
     hwaddr pa = 0;
     int prot;
-#endif
     int ret = TRANSLATE_FAIL;
 
-    qemu_log_mask(CPU_LOG_MMU,
-            "%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx \
-             %d\n", __func__, env->pc, address, rw, mmu_idx);
+    qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
+                  __func__, address, access_type, mmu_idx);
+
+    ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
 
-#if !defined(CONFIG_USER_ONLY)
-    ret = get_physical_address(env, &pa, &prot, address, rw, mmu_idx);
     qemu_log_mask(CPU_LOG_MMU,
-            "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
-             " prot %d\n", __func__, address, ret, pa, prot);
+                  "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
+                  " prot %d\n", __func__, address, ret, pa, prot);
+
     if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-        !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << rw)) {
+        !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type)) {
         ret = TRANSLATE_FAIL;
     }
     if (ret == TRANSLATE_SUCCESS) {
         tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
                      prot, mmu_idx, TARGET_PAGE_SIZE);
-    } else if (ret == TRANSLATE_FAIL) {
-        raise_mmu_exception(env, address, rw);
+        return true;
+    } else if (probe) {
+        return false;
+    } else {
+        raise_mmu_exception(env, address, access_type);
+        riscv_raise_exception(env, cs->exception_index, retaddr);
     }
 #else
-    switch (rw) {
+    switch (access_type) {
     case MMU_INST_FETCH:
         cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
         break;
@@ -436,8 +432,8 @@ int riscv_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
         cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
         break;
     }
+    cpu_loop_exit_restore(cs, retaddr);
 #endif
-    return ret;
 }
 
 /*
-- 
2.17.1

[Qemu-riscv] [PULL v2 17/27] target/riscv: Convert to CPUClass::tlb_fill

[Richard Henderson]

Note that env->pc is removed from the qemu_log as that value is garbage.
The PC isn't recovered until cpu_restore_state, called from
cpu_loop_exit_restore, called from riscv_raise_exception.

Cc: qemu-riscv@nongnu.org
Cc: Palmer Dabbelt <palmer@sifive.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/riscv/cpu.h        |  5 +++--
 target/riscv/cpu.c        |  5 ++---
 target/riscv/cpu_helper.c | 46 ++++++++++++++++++---------------------
 3 files changed, 26 insertions(+), 30 deletions(-)

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 7d9f48973f..c17184f4e4 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -261,8 +261,9 @@ hwaddr riscv_cpu_get_phys_page_debug(CPUState *cpu, vaddr addr);
 void  riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
                                     MMUAccessType access_type, int mmu_idx,
                                     uintptr_t retaddr);
-int riscv_cpu_handle_mmu_fault(CPUState *cpu, vaddr address, int size,
-                              int rw, int mmu_idx);
+bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                        MMUAccessType access_type, int mmu_idx,
+                        bool probe, uintptr_t retaddr);
 char *riscv_isa_string(RISCVCPU *cpu);
 void riscv_cpu_list(void);
 
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 1bcf4eaeb8..b7675707e0 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -355,14 +355,13 @@ static void riscv_cpu_class_init(ObjectClass *c, void *data)
 #endif
     cc->gdb_stop_before_watchpoint = true;
     cc->disas_set_info = riscv_cpu_disas_set_info;
-#ifdef CONFIG_USER_ONLY
-    cc->handle_mmu_fault = riscv_cpu_handle_mmu_fault;
-#else
+#ifndef CONFIG_USER_ONLY
     cc->do_unaligned_access = riscv_cpu_do_unaligned_access;
     cc->get_phys_page_debug = riscv_cpu_get_phys_page_debug;
 #endif
 #ifdef CONFIG_TCG
     cc->tcg_initialize = riscv_translate_init;
+    cc->tlb_fill = riscv_cpu_tlb_fill;
 #endif
     /* For now, mark unmigratable: */
     cc->vmsd = &vmstate_riscv_cpu;
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index b17f169681..2535435260 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -379,53 +379,49 @@ void riscv_cpu_do_unaligned_access(CPUState *cs, vaddr addr,
     riscv_raise_exception(env, cs->exception_index, retaddr);
 }
 
-/* called by qemu's softmmu to fill the qemu tlb */
 void tlb_fill(CPUState *cs, target_ulong addr, int size,
         MMUAccessType access_type, int mmu_idx, uintptr_t retaddr)
 {
-    int ret;
-    ret = riscv_cpu_handle_mmu_fault(cs, addr, size, access_type, mmu_idx);
-    if (ret == TRANSLATE_FAIL) {
-        RISCVCPU *cpu = RISCV_CPU(cs);
-        CPURISCVState *env = &cpu->env;
-        riscv_raise_exception(env, cs->exception_index, retaddr);
-    }
+    riscv_cpu_tlb_fill(cs, addr, size, access_type, mmu_idx, false, retaddr);
 }
-
 #endif
 
-int riscv_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
-        int rw, int mmu_idx)
+bool riscv_cpu_tlb_fill(CPUState *cs, vaddr address, int size,
+                        MMUAccessType access_type, int mmu_idx,
+                        bool probe, uintptr_t retaddr)
 {
+#ifndef CONFIG_USER_ONLY
     RISCVCPU *cpu = RISCV_CPU(cs);
     CPURISCVState *env = &cpu->env;
-#if !defined(CONFIG_USER_ONLY)
     hwaddr pa = 0;
     int prot;
-#endif
     int ret = TRANSLATE_FAIL;
 
-    qemu_log_mask(CPU_LOG_MMU,
-            "%s pc " TARGET_FMT_lx " ad %" VADDR_PRIx " rw %d mmu_idx \
-             %d\n", __func__, env->pc, address, rw, mmu_idx);
+    qemu_log_mask(CPU_LOG_MMU, "%s ad %" VADDR_PRIx " rw %d mmu_idx %d\n",
+                  __func__, address, access_type, mmu_idx);
+
+    ret = get_physical_address(env, &pa, &prot, address, access_type, mmu_idx);
 
-#if !defined(CONFIG_USER_ONLY)
-    ret = get_physical_address(env, &pa, &prot, address, rw, mmu_idx);
     qemu_log_mask(CPU_LOG_MMU,
-            "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
-             " prot %d\n", __func__, address, ret, pa, prot);
+                  "%s address=%" VADDR_PRIx " ret %d physical " TARGET_FMT_plx
+                  " prot %d\n", __func__, address, ret, pa, prot);
+
     if (riscv_feature(env, RISCV_FEATURE_PMP) &&
-        !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << rw)) {
+        !pmp_hart_has_privs(env, pa, TARGET_PAGE_SIZE, 1 << access_type)) {
         ret = TRANSLATE_FAIL;
     }
     if (ret == TRANSLATE_SUCCESS) {
         tlb_set_page(cs, address & TARGET_PAGE_MASK, pa & TARGET_PAGE_MASK,
                      prot, mmu_idx, TARGET_PAGE_SIZE);
-    } else if (ret == TRANSLATE_FAIL) {
-        raise_mmu_exception(env, address, rw);
+        return true;
+    } else if (probe) {
+        return false;
+    } else {
+        raise_mmu_exception(env, address, access_type);
+        riscv_raise_exception(env, cs->exception_index, retaddr);
     }
 #else
-    switch (rw) {
+    switch (access_type) {
     case MMU_INST_FETCH:
         cs->exception_index = RISCV_EXCP_INST_PAGE_FAULT;
         break;
@@ -436,8 +432,8 @@ int riscv_cpu_handle_mmu_fault(CPUState *cs, vaddr address, int size,
         cs->exception_index = RISCV_EXCP_STORE_PAGE_FAULT;
         break;
     }
+    cpu_loop_exit_restore(cs, retaddr);
 #endif
-    return ret;
 }
 
 /*
-- 
2.17.1

[Qemu-devel] [PULL v2 18/27] target/s390x: Convert to CPUClass::tlb_fill

[Richard Henderson]

Cc: qemu-s390x@nongnu.org
Cc: Cornelia Huck <cohuck@redhat.com>
Reviewed-by: David Hildenbrand <david@redhat.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 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..b1df63d82c 100644
--- a/target/s390x/cpu.c
+++ b/target/s390x/cpu.c
@@ -478,9 +478,7 @@ 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
+#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;
@@ -493,6 +491,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data)
     cc->disas_set_info = s390_cpu_disas_set_info;
 #ifdef CONFIG_TCG
     cc->tcg_initialize = s390x_translate_init;
+    cc->tlb_fill = s390_cpu_tlb_fill;
 #endif
 
     cc->gdb_num_core_regs = S390_NUM_CORE_REGS;
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
-- 
2.17.1

[Qemu-devel] [PULL v2 27/27] tcg: Use tlb_fill probe from tlb_vaddr_to_host

[Richard Henderson]

Most of the existing users would continue around a loop which
would fault the tlb entry in via a normal load/store.

But for AArch64 SVE we have an existing emulation bug wherein we
would mark the first element of a no-fault vector load as faulted
(within the FFR, not via exception) just because we did not have
its address in the TLB.  Now we can properly only mark it as faulted
if there really is no valid, readable translation, while still not
raising an exception.  (Note that beyond the first element of the
vector, the hardware may report a fault for any reason whatsoever;
with at least one element loaded, forward progress is guaranteed.)

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu_ldst.h | 50 ++++++-----------------------
 accel/tcg/cputlb.c      | 69 ++++++++++++++++++++++++++++++++++++-----
 target/arm/sve_helper.c |  6 +---
 3 files changed, 72 insertions(+), 53 deletions(-)

diff --git a/include/exec/cpu_ldst.h b/include/exec/cpu_ldst.h
index d78041d7a0..7b28a839d2 100644
--- a/include/exec/cpu_ldst.h
+++ b/include/exec/cpu_ldst.h
@@ -433,50 +433,20 @@ static inline CPUTLBEntry *tlb_entry(CPUArchState *env, uintptr_t mmu_idx,
  * @mmu_idx: MMU index to use for lookup
  *
  * Look up the specified guest virtual index in the TCG softmmu TLB.
- * If the TLB contains a host virtual address suitable for direct RAM
- * access, then return it. Otherwise (TLB miss, TLB entry is for an
- * I/O access, etc) return NULL.
- *
- * This is the equivalent of the initial fast-path code used by
- * TCG backends for guest load and store accesses.
+ * If we can translate a host virtual address suitable for direct RAM
+ * access, without causing a guest exception, then return it.
+ * Otherwise (TLB entry is for an I/O access, guest software
+ * TLB fill required, etc) return NULL.
  */
+#ifdef CONFIG_USER_ONLY
 static inline void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
-                                      int access_type, int mmu_idx)
+                                      MMUAccessType access_type, int mmu_idx)
 {
-#if defined(CONFIG_USER_ONLY)
     return g2h(addr);
-#else
-    CPUTLBEntry *tlbentry = tlb_entry(env, mmu_idx, addr);
-    abi_ptr tlb_addr;
-    uintptr_t haddr;
-
-    switch (access_type) {
-    case 0:
-        tlb_addr = tlbentry->addr_read;
-        break;
-    case 1:
-        tlb_addr = tlb_addr_write(tlbentry);
-        break;
-    case 2:
-        tlb_addr = tlbentry->addr_code;
-        break;
-    default:
-        g_assert_not_reached();
-    }
-
-    if (!tlb_hit(tlb_addr, addr)) {
-        /* TLB entry is for a different page */
-        return NULL;
-    }
-
-    if (tlb_addr & ~TARGET_PAGE_MASK) {
-        /* IO access */
-        return NULL;
-    }
-
-    haddr = addr + tlbentry->addend;
-    return (void *)haddr;
-#endif /* defined(CONFIG_USER_ONLY) */
 }
+#else
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+                        MMUAccessType access_type, int mmu_idx);
+#endif
 
 #endif /* CPU_LDST_H */
diff --git a/accel/tcg/cputlb.c b/accel/tcg/cputlb.c
index dfcd9ae168..685e0f2ee4 100644
--- a/accel/tcg/cputlb.c
+++ b/accel/tcg/cputlb.c
@@ -1007,6 +1007,16 @@ static void io_writex(CPUArchState *env, CPUIOTLBEntry *iotlbentry,
     }
 }
 
+static inline target_ulong tlb_read_ofs(CPUTLBEntry *entry, size_t ofs)
+{
+#if TCG_OVERSIZED_GUEST
+    return *(target_ulong *)((uintptr_t)entry + ofs);
+#else
+    /* ofs might correspond to .addr_write, so use atomic_read */
+    return atomic_read((target_ulong *)((uintptr_t)entry + ofs));
+#endif
+}
+
 /* Return true if ADDR is present in the victim tlb, and has been copied
    back to the main tlb.  */
 static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index,
@@ -1017,14 +1027,7 @@ static bool victim_tlb_hit(CPUArchState *env, size_t mmu_idx, size_t index,
     assert_cpu_is_self(ENV_GET_CPU(env));
     for (vidx = 0; vidx < CPU_VTLB_SIZE; ++vidx) {
         CPUTLBEntry *vtlb = &env->tlb_v_table[mmu_idx][vidx];
-        target_ulong cmp;
-
-        /* elt_ofs might correspond to .addr_write, so use atomic_read */
-#if TCG_OVERSIZED_GUEST
-        cmp = *(target_ulong *)((uintptr_t)vtlb + elt_ofs);
-#else
-        cmp = atomic_read((target_ulong *)((uintptr_t)vtlb + elt_ofs));
-#endif
+        target_ulong cmp = tlb_read_ofs(vtlb, elt_ofs);
 
         if (cmp == page) {
             /* Found entry in victim tlb, swap tlb and iotlb.  */
@@ -1108,6 +1111,56 @@ void probe_write(CPUArchState *env, target_ulong addr, int size, int mmu_idx,
     }
 }
 
+void *tlb_vaddr_to_host(CPUArchState *env, abi_ptr addr,
+                        MMUAccessType access_type, int mmu_idx)
+{
+    CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
+    uintptr_t tlb_addr, page;
+    size_t elt_ofs;
+
+    switch (access_type) {
+    case MMU_DATA_LOAD:
+        elt_ofs = offsetof(CPUTLBEntry, addr_read);
+        break;
+    case MMU_DATA_STORE:
+        elt_ofs = offsetof(CPUTLBEntry, addr_write);
+        break;
+    case MMU_INST_FETCH:
+        elt_ofs = offsetof(CPUTLBEntry, addr_code);
+        break;
+    default:
+        g_assert_not_reached();
+    }
+
+    page = addr & TARGET_PAGE_MASK;
+    tlb_addr = tlb_read_ofs(entry, elt_ofs);
+
+    if (!tlb_hit_page(tlb_addr, page)) {
+        uintptr_t index = tlb_index(env, mmu_idx, addr);
+
+        if (!victim_tlb_hit(env, mmu_idx, index, elt_ofs, page)) {
+            CPUState *cs = ENV_GET_CPU(env);
+            CPUClass *cc = CPU_GET_CLASS(cs);
+
+            if (!cc->tlb_fill(cs, addr, 0, access_type, mmu_idx, true, 0)) {
+                /* Non-faulting page table read failed.  */
+                return NULL;
+            }
+
+            /* TLB resize via tlb_fill may have moved the entry.  */
+            entry = tlb_entry(env, mmu_idx, addr);
+        }
+        tlb_addr = tlb_read_ofs(entry, elt_ofs);
+    }
+
+    if (tlb_addr & ~TARGET_PAGE_MASK) {
+        /* IO access */
+        return NULL;
+    }
+
+    return (void *)((uintptr_t)addr + entry->addend);
+}
+
 /* Probe for a read-modify-write atomic operation.  Do not allow unaligned
  * operations, or io operations to proceed.  Return the host address.  */
 static void *atomic_mmu_lookup(CPUArchState *env, target_ulong addr,
diff --git a/target/arm/sve_helper.c b/target/arm/sve_helper.c
index bc847250dd..fd434c66ea 100644
--- a/target/arm/sve_helper.c
+++ b/target/arm/sve_helper.c
@@ -4598,11 +4598,7 @@ static void sve_ldnf1_r(CPUARMState *env, void *vg, const target_ulong addr,
      * in the real world, obviously.)
      *
      * Then there are the annoying special cases with watchpoints...
-     *
-     * TODO: Add a form of tlb_fill that does not raise an exception,
-     * with a form of tlb_vaddr_to_host and a set of loads to match.
-     * The non_fault_vaddr_to_host would handle everything, usually,
-     * and the loads would handle the iomem path for watchpoints.
+     * TODO: Add a form of non-faulting loads using cc->tlb_fill(probe=true).
      */
     host = tlb_vaddr_to_host(env, addr + mem_off, MMU_DATA_LOAD, mmu_idx);
     split = max_for_page(addr, mem_off, mem_max);
-- 
2.17.1

Re: [Qemu-devel] [PULL v2 00/27] tcg: Add CPUClass::tlb_fill

[Aleksandar Markovic]

On May 10, 2019 8:55 PM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>
> Changes in v2:
>
>   * Fix --disable-tcg compilation for x86 and s390x.
>     I adjusted every target/ that used any CONFIG_TCG in cpu.c.
>     but then afterward I see that only x86 and s390x have had
>     their Makefiles adjusted to make --disable-tcg actually work.
>
>   * Fix Werror for 64-bit on 32-bit.
>
> Only re-posting changed patches.
>

Hello, Richard.

Just want to stress that there are unaddressed concerns for patch 12/27.

Thanks,
Aleksamdar

>
> r~
>
>
> The following changes since commit
efb4f3b62c69383a7308d7b739a3193e7c0ccae8:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request'
into staging (2019-05-10 14:49:36 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/rth7680/qemu.git tags/pull-tcg-20190510
>
> for you to fetch changes up to 4811e9095c0491bc6f5450e5012c9c4796b9e59d:
>
>   tcg: Use tlb_fill probe from tlb_vaddr_to_host (2019-05-10 11:12:50
-0700)
>
> ----------------------------------------------------------------
> Add CPUClass::tlb_fill.
> Improve tlb_vaddr_to_host for use by ARM SVE no-fault loads.
>
> ----------------------------------------------------------------
> Richard Henderson (27):
>       tcg: Add CPUClass::tlb_fill
>       target/alpha: Convert to CPUClass::tlb_fill
>       target/arm: Convert to CPUClass::tlb_fill
>       target/cris: Convert to CPUClass::tlb_fill
>       target/hppa: Convert to CPUClass::tlb_fill
>       target/i386: Convert to CPUClass::tlb_fill
>       target/lm32: Convert to CPUClass::tlb_fill
>       target/m68k: Convert to CPUClass::tlb_fill
>       target/microblaze: Convert to CPUClass::tlb_fill
>       target/mips: Pass a valid error to raise_mmu_exception for user-only
>       target/mips: Tidy control flow in mips_cpu_handle_mmu_fault
>       target/mips: Convert to CPUClass::tlb_fill
>       target/moxie: Convert to CPUClass::tlb_fill
>       target/nios2: Convert to CPUClass::tlb_fill
>       target/openrisc: Convert to CPUClass::tlb_fill
>       target/ppc: Convert to CPUClass::tlb_fill
>       target/riscv: Convert to CPUClass::tlb_fill
>       target/s390x: Convert to CPUClass::tlb_fill
>       target/sh4: Convert to CPUClass::tlb_fill
>       target/sparc: Convert to CPUClass::tlb_fill
>       target/tilegx: Convert to CPUClass::tlb_fill
>       target/tricore: Convert to CPUClass::tlb_fill
>       target/unicore32: Convert to CPUClass::tlb_fill
>       target/xtensa: Convert to CPUClass::tlb_fill
>       tcg: Use CPUClass::tlb_fill in cputlb.c
>       tcg: Remove CPUClass::handle_mmu_fault
>       tcg: Use tlb_fill probe from tlb_vaddr_to_host
>
>  include/exec/cpu_ldst.h         |  50 +++--------
>  include/exec/exec-all.h         |   9 --
>  include/qom/cpu.h               |  12 ++-
>  target/alpha/cpu.h              |   5 +-
>  target/arm/internals.h          |  10 ++-
>  target/cris/cpu.h               |   5 +-
>  target/hppa/cpu.h               |   8 +-
>  target/i386/cpu.h               |   5 +-
>  target/lm32/cpu.h               |   5 +-
>  target/m68k/cpu.h               |   5 +-
>  target/microblaze/cpu.h         |   5 +-
>  target/mips/internal.h          |   5 +-
>  target/moxie/cpu.h              |   5 +-
>  target/nios2/cpu.h              |   5 +-
>  target/openrisc/cpu.h           |   5 +-
>  target/ppc/cpu.h                |   7 +-
>  target/riscv/cpu.h              |   5 +-
>  target/s390x/internal.h         |   5 +-
>  target/sh4/cpu.h                |   5 +-
>  target/sparc/cpu.h              |   5 +-
>  target/tricore/cpu.h            |   6 +-
>  target/unicore32/cpu.h          |   5 +-
>  target/xtensa/cpu.h             |   5 +-
>  accel/tcg/cputlb.c              |  88 +++++++++++++++++--
>  accel/tcg/user-exec.c           |  36 ++------
>  target/alpha/cpu.c              |   5 +-
>  target/alpha/helper.c           |  24 +++--
>  target/alpha/mem_helper.c       |  16 ----
>  target/arm/cpu.c                |  22 +----
>  target/arm/helper.c             |  90 +++++++++++--------
>  target/arm/op_helper.c          |  29 +-----
>  target/arm/sve_helper.c         |   6 +-
>  target/cris/cpu.c               |   5 +-
>  target/cris/helper.c            |  61 ++++++-------
>  target/cris/op_helper.c         |  28 ------
>  target/hppa/cpu.c               |   5 +-
>  target/hppa/mem_helper.c        |  16 ++--
>  target/i386/cpu.c               |   5 +-
>  target/i386/excp_helper.c       |  53 ++++++-----
>  target/i386/mem_helper.c        |  21 -----
>  target/lm32/cpu.c               |   5 +-
>  target/lm32/helper.c            |   8 +-
>  target/lm32/op_helper.c         |  16 ----
>  target/m68k/cpu.c               |   2 +-
>  target/m68k/helper.c            |  89 +++++++++----------
>  target/m68k/op_helper.c         |  15 ----
>  target/microblaze/cpu.c         |   5 +-
>  target/microblaze/helper.c      | 101 ++++++++++-----------
>  target/microblaze/op_helper.c   |  19 ----
>  target/mips/cpu.c               |   5 +-
>  target/mips/helper.c            |  81 ++++++++---------
>  target/mips/op_helper.c         |  15 ----
>  target/moxie/cpu.c              |   5 +-
>  target/moxie/helper.c           |  65 +++-----------
>  target/nios2/cpu.c              |   5 +-
>  target/nios2/helper.c           | 170
+++++++++++++++++-------------------
>  target/nios2/mmu.c              |  12 ---
>  target/openrisc/cpu.c           |   5 +-
>  target/openrisc/mmu.c           |  69 +++++++--------
>  target/ppc/mmu_helper.c         |  16 ++--
>  target/ppc/translate_init.inc.c |   5 +-
>  target/ppc/user_only_helper.c   |  14 +--
>  target/riscv/cpu.c              |   5 +-
>  target/riscv/cpu_helper.c       |  50 +++++------
>  target/s390x/cpu.c              |   5 +-
>  target/s390x/excp_helper.c      |  67 +++++++++-----
>  target/s390x/mem_helper.c       |  16 ----
>  target/sh4/cpu.c                |   5 +-
>  target/sh4/helper.c             | 189
+++++++++++++++++++---------------------
>  target/sh4/op_helper.c          |  12 ---
>  target/sparc/cpu.c              |   5 +-
>  target/sparc/ldst_helper.c      |  15 ----
>  target/sparc/mmu_helper.c       |  58 +++++++-----
>  target/tilegx/cpu.c             |  10 ++-
>  target/tricore/cpu.c            |   1 +
>  target/tricore/helper.c         |  23 +++--
>  target/tricore/op_helper.c      |  26 ------
>  target/unicore32/cpu.c          |   5 +-
>  target/unicore32/helper.c       |  23 -----
>  target/unicore32/op_helper.c    |  14 ---
>  target/unicore32/softmmu.c      |  13 ++-
>  target/xtensa/cpu.c             |   5 +-
>  target/xtensa/helper.c          |  33 ++++---
>  83 files changed, 868 insertions(+), 1131 deletions(-)
>

Re: [Qemu-devel] [PULL v2 00/27] tcg: Add CPUClass::tlb_fill

[Peter Maydell]

On Fri, 10 May 2019 at 19:54, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Changes in v2:
>
>   * Fix --disable-tcg compilation for x86 and s390x.
>     I adjusted every target/ that used any CONFIG_TCG in cpu.c.
>     but then afterward I see that only x86 and s390x have had
>     their Makefiles adjusted to make --disable-tcg actually work.
>
>   * Fix Werror for 64-bit on 32-bit.
>
> Only re-posting changed patches.
>
>
> r~
>
>
> The following changes since commit efb4f3b62c69383a7308d7b739a3193e7c0ccae8:
>
>   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-05-10 14:49:36 +0100)
>
> are available in the Git repository at:
>
>   https://github.com/rth7680/qemu.git tags/pull-tcg-20190510
>
> for you to fetch changes up to 4811e9095c0491bc6f5450e5012c9c4796b9e59d:
>
>   tcg: Use tlb_fill probe from tlb_vaddr_to_host (2019-05-10 11:12:50 -0700)
>
> ----------------------------------------------------------------
> Add CPUClass::tlb_fill.
> Improve tlb_vaddr_to_host for use by ARM SVE no-fault loads.
>

I did a test merge of this and it passed my build tests; I haven't
applied it though pending resolution of Aleksandar's comments.

thanks
-- PMM

Re: [Qemu-devel] [PULL v2 00/27] tcg: Add CPUClass::tlb_fill

[Peter Maydell]

On Tue, 14 May 2019 at 13:36, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 10 May 2019 at 19:54, Richard Henderson
> <richard.henderson@linaro.org> wrote:
> >
> > Changes in v2:
> >
> >   * Fix --disable-tcg compilation for x86 and s390x.
> >     I adjusted every target/ that used any CONFIG_TCG in cpu.c.
> >     but then afterward I see that only x86 and s390x have had
> >     their Makefiles adjusted to make --disable-tcg actually work.
> >
> >   * Fix Werror for 64-bit on 32-bit.
> >
> > Only re-posting changed patches.
> >
> >
> > r~
> >
> >
> > The following changes since commit efb4f3b62c69383a7308d7b739a3193e7c0ccae8:
> >
> >   Merge remote-tracking branch 'remotes/stefanha/tags/block-pull-request' into staging (2019-05-10 14:49:36 +0100)
> >
> > are available in the Git repository at:
> >
> >   https://github.com/rth7680/qemu.git tags/pull-tcg-20190510
> >
> > for you to fetch changes up to 4811e9095c0491bc6f5450e5012c9c4796b9e59d:
> >
> >   tcg: Use tlb_fill probe from tlb_vaddr_to_host (2019-05-10 11:12:50 -0700)
> >
> > ----------------------------------------------------------------
> > Add CPUClass::tlb_fill.
> > Improve tlb_vaddr_to_host for use by ARM SVE no-fault loads.
> >
>
> I did a test merge of this and it passed my build tests; I haven't
> applied it though pending resolution of Aleksandar's comments.


Applied, thanks (conclusion of mailing list discussion was that it
was ok to apply as-is).

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

-- PMM