qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Clean up MMU translation
@ 2021-06-21 12:51 Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, richard.henderson, luis.pires, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

This is the second half of rth's patch cleaning up MMU address
translation, which is not complete but is a good start! This patch
series is also required to finally support disable-tcg.

The final patch fixes the bug mentioned by rth and farosas, that cedric
did not confirm actually happens, but agreed is a good cleanup either
way.

Changes for v2:
* rebased on ppc-for-6.1
* added the bugfix

Bruno Larsen (billionai) (1):
  target/ppc: fix address translation bug for radix mmus

Richard Henderson (9):
  target/ppc: Remove PowerPCCPUClass.handle_mmu_fault
  target/ppc: Use MMUAccessType with *_handle_mmu_fault
  target/ppc: Push real-mode handling into ppc_radix64_xlate
  target/ppc: Use bool success for ppc_radix64_xlate
  target/ppc: Split out ppc_hash64_xlate
  target/ppc: Split out ppc_hash32_xlate
  target/ppc: Split out ppc_jumbo_xlate
  target/ppc: Introduce ppc_xlate
  target/ppc: Restrict ppc_cpu_tlb_fill to TCG

 target/ppc/cpu-qom.h       |   1 -
 target/ppc/cpu_init.c      |  45 --------
 target/ppc/internal.h      |  13 +++
 target/ppc/mmu-book3s-v3.c |  19 ----
 target/ppc/mmu-book3s-v3.h |   5 -
 target/ppc/mmu-hash32.c    | 217 ++++++++++++++++---------------------
 target/ppc/mmu-hash32.h    |   6 +-
 target/ppc/mmu-hash64.c    | 118 +++++++-------------
 target/ppc/mmu-hash64.h    |   6 +-
 target/ppc/mmu-radix64.c   | 152 ++++++++++----------------
 target/ppc/mmu-radix64.h   |   6 +-
 target/ppc/mmu_helper.c    | 201 +++++++++++++++++-----------------
 12 files changed, 313 insertions(+), 476 deletions(-)

-- 
2.17.1



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

* [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-22 10:49   ` Greg Kurz
  2021-06-24  1:40   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Instead, use a switch on env->mmu_model.  This avoids some
replicated information in cpu setup.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/cpu-qom.h    |  1 -
 target/ppc/cpu_init.c   | 45 -----------------------------------------
 target/ppc/mmu_helper.c | 24 ++++++++++++++++++----
 3 files changed, 20 insertions(+), 50 deletions(-)

diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 06b6571bc9..3b14d2f134 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -198,7 +198,6 @@ struct PowerPCCPUClass {
     int n_host_threads;
     void (*init_proc)(CPUPPCState *env);
     int  (*check_pow)(CPUPPCState *env);
-    int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int mmu_idx);
     bool (*interrupts_big_endian)(PowerPCCPU *cpu);
 };
 
diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index d0411e7302..3a8d8d3f07 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -4578,9 +4578,6 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
                     (1ull << MSR_IR) |
                     (1ull << MSR_DR);
     pcc->mmu_model = POWERPC_MMU_601;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_601;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_601;
@@ -4623,9 +4620,6 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
                     (1ull << MSR_IR) |
                     (1ull << MSR_DR);
     pcc->mmu_model = POWERPC_MMU_601;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_601;
     pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
@@ -4889,9 +4883,6 @@ POWERPC_FAMILY(604)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_604;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_604;
@@ -4973,9 +4964,6 @@ POWERPC_FAMILY(604E)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_604;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_604;
@@ -5044,9 +5032,6 @@ POWERPC_FAMILY(740)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5124,9 +5109,6 @@ POWERPC_FAMILY(750)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5327,9 +5309,6 @@ POWERPC_FAMILY(750cl)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5410,9 +5389,6 @@ POWERPC_FAMILY(750cx)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5498,9 +5474,6 @@ POWERPC_FAMILY(750fx)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5586,9 +5559,6 @@ POWERPC_FAMILY(750gx)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_7x0;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_750;
@@ -5828,9 +5798,6 @@ POWERPC_FAMILY(7400)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_74xx;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_7400;
@@ -5914,9 +5881,6 @@ POWERPC_FAMILY(7410)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_74xx;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_7400;
@@ -6743,9 +6707,6 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI) |
                     (1ull << MSR_LE);
     pcc->mmu_model = POWERPC_MMU_32B;
-#if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
-#endif
     pcc->excp_model = POWERPC_EXCP_74xx;
     pcc->bus_model = PPC_FLAGS_INPUT_6xx;
     pcc->bfd_mach = bfd_mach_ppc_7400;
@@ -7505,7 +7466,6 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
                     (1ull << MSR_RI);
     pcc->mmu_model = POWERPC_MMU_64B;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
     pcc->hash64_opts = &ppc_hash64_opts_basic;
 #endif
     pcc->excp_model = POWERPC_EXCP_970;
@@ -7583,7 +7543,6 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
         LPCR_RMI | LPCR_HDICE;
     pcc->mmu_model = POWERPC_MMU_2_03;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
     pcc->hash64_opts = &ppc_hash64_opts_basic;
     pcc->lrg_decr_bits = 32;
 #endif
@@ -7727,7 +7686,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
     pcc->lpcr_pm = LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2;
     pcc->mmu_model = POWERPC_MMU_2_06;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
     pcc->hash64_opts = &ppc_hash64_opts_POWER7;
     pcc->lrg_decr_bits = 32;
 #endif
@@ -7904,7 +7862,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
                    LPCR_P8_PECE3 | LPCR_P8_PECE4;
     pcc->mmu_model = POWERPC_MMU_2_07;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
     pcc->hash64_opts = &ppc_hash64_opts_POWER7;
     pcc->lrg_decr_bits = 32;
     pcc->n_host_threads = 8;
@@ -8120,7 +8077,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
     pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
     pcc->mmu_model = POWERPC_MMU_3_00;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
     /* segment page size remain the same */
     pcc->hash64_opts = &ppc_hash64_opts_POWER7;
     pcc->radix_page_info = &POWER9_radix_page_info;
@@ -8332,7 +8288,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
     pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
     pcc->mmu_model = POWERPC_MMU_3_00;
 #if defined(CONFIG_SOFTMMU)
-    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
     /* segment page size remain the same */
     pcc->hash64_opts = &ppc_hash64_opts_POWER7;
     pcc->radix_page_info = &POWER10_radix_page_info;
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 1ecb36e85a..c4b1c93e47 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2947,14 +2947,30 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
                       bool probe, uintptr_t retaddr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
-    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
     CPUPPCState *env = &cpu->env;
     int ret;
 
-    if (pcc->handle_mmu_fault) {
-        ret = pcc->handle_mmu_fault(cpu, addr, access_type, mmu_idx);
-    } else {
+    switch (env->mmu_model) {
+#if defined(TARGET_PPC64)
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_03:
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_07:
+        ret = ppc_hash64_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
+        break;
+    case POWERPC_MMU_3_00:
+        ret = ppc64_v3_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
+        break;
+#endif
+
+    case POWERPC_MMU_32B:
+    case POWERPC_MMU_601:
+        ret = ppc_hash32_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
+        break;
+
+    default:
         ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
+        break;
     }
     if (unlikely(ret != 0)) {
         if (probe) {
-- 
2.17.1



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

* [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-22 12:05   ` Greg Kurz
  2021-06-24  3:19   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate Bruno Larsen (billionai)
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

These changes were waiting until we didn't need to match
the function type of PowerPCCPUClass.handle_mmu_fault.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-hash32.c  | 7 ++-----
 target/ppc/mmu-hash32.h  | 4 ++--
 target/ppc/mmu-hash64.c  | 6 +-----
 target/ppc/mmu-hash64.h  | 4 ++--
 target/ppc/mmu-radix64.c | 7 ++-----
 target/ppc/mmu-radix64.h | 4 ++--
 6 files changed, 11 insertions(+), 21 deletions(-)

diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 9f0a497657..8f19b43e47 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -415,8 +415,8 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
     return (rpn & ~mask) | (eaddr & mask);
 }
 
-int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
-                                int mmu_idx)
+int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                MMUAccessType access_type, int mmu_idx)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -425,11 +425,8 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
     ppc_hash_pte32_t pte;
     int prot;
     int need_prot;
-    MMUAccessType access_type;
     hwaddr raddr;
 
-    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
-    access_type = rwx;
     need_prot = prot_for_access_type(access_type);
 
     /* 1. Handle real mode accesses */
diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
index 898021f0d8..30e35718a7 100644
--- a/target/ppc/mmu-hash32.h
+++ b/target/ppc/mmu-hash32.h
@@ -5,8 +5,8 @@
 
 hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
 hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
-int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
-                                int mmu_idx);
+int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
+                                MMUAccessType access_type, int mmu_idx);
 
 /*
  * Segment register definitions
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 708dffc31b..2febd369b1 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -874,7 +874,7 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
 }
 
 int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                int rwx, int mmu_idx)
+                                MMUAccessType access_type, int mmu_idx)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -884,13 +884,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
     hwaddr ptex;
     ppc_hash_pte64_t pte;
     int exec_prot, pp_prot, amr_prot, prot;
-    MMUAccessType access_type;
     int need_prot;
     hwaddr raddr;
 
-    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
-    access_type = rwx;
-
     /*
      * Note on LPCR usage: 970 uses HID4, but our special variant of
      * store_spr copies relevant fields into env->spr[SPR_LPCR].
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 4b8b8e7950..3e8a8eec1f 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -8,8 +8,8 @@ void dump_slb(PowerPCCPU *cpu);
 int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
                   target_ulong esid, target_ulong vsid);
 hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
-int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
-                                int mmu_idx);
+int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
+                                MMUAccessType access_type, int mmu_idx);
 void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
                                target_ulong pte_index,
                                target_ulong pte0, target_ulong pte1);
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index b6d191c1d8..1c707d387d 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -555,19 +555,16 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
     return 0;
 }
 
-int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
-                                 int mmu_idx)
+int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                 MMUAccessType access_type, int mmu_idx)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     int page_size, prot;
     bool relocation;
-    MMUAccessType access_type;
     hwaddr raddr;
 
     assert(!(msr_hv && cpu->vhyp));
-    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
-    access_type = rwx;
 
     relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
     /* HV or virtual hypervisor Real Mode Access */
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index f28c5794d0..94bd72cb38 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -44,8 +44,8 @@
 
 #ifdef TARGET_PPC64
 
-int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
-                                 int mmu_idx);
+int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                 MMUAccessType access_type, int mmu_idx);
 hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
-- 
2.17.1



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

* [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  3:29   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate Bruno Larsen (billionai)
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

This removes some incomplete duplication between
ppc_radix64_handle_mmu_fault and ppc_radix64_get_phys_page_debug.
The former was correct wrt SPR_HRMOR and the latter was not.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-radix64.c | 77 ++++++++++++++++++----------------------
 1 file changed, 34 insertions(+), 43 deletions(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 1c707d387d..dd5ae69052 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -465,7 +465,6 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
  */
 static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
                              MMUAccessType access_type,
-                             bool relocation,
                              hwaddr *raddr, int *psizep, int *protp,
                              bool guest_visible)
 {
@@ -474,6 +473,37 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
     ppc_v3_pate_t pate;
     int psize, prot;
     hwaddr g_raddr;
+    bool relocation;
+
+    assert(!(msr_hv && cpu->vhyp));
+
+    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
+
+    /* HV or virtual hypervisor Real Mode Access */
+    if (!relocation && (msr_hv || cpu->vhyp)) {
+        /* In real mode top 4 effective addr bits (mostly) ignored */
+        *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
+
+        /* In HV mode, add HRMOR if top EA bit is clear */
+        if (msr_hv || !env->has_hv_mode) {
+            if (!(eaddr >> 63)) {
+                *raddr |= env->spr[SPR_HRMOR];
+           }
+        }
+        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+        *psizep = TARGET_PAGE_BITS;
+        return 0;
+    }
+
+    /*
+     * Check UPRT (we avoid the check in real mode to deal with
+     * transitional states during kexec.
+     */
+    if (guest_visible && !ppc64_use_proc_tbl(cpu)) {
+        qemu_log_mask(LOG_GUEST_ERROR,
+                      "LPCR:UPRT not set in radix mode ! LPCR="
+                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
+    }
 
     /* Virtual Mode Access - get the fully qualified address */
     if (!ppc_radix64_get_fully_qualified_addr(&cpu->env, eaddr, &lpid, &pid)) {
@@ -559,43 +589,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
                                  MMUAccessType access_type, int mmu_idx)
 {
     CPUState *cs = CPU(cpu);
-    CPUPPCState *env = &cpu->env;
     int page_size, prot;
-    bool relocation;
     hwaddr raddr;
 
-    assert(!(msr_hv && cpu->vhyp));
-
-    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
-    /* HV or virtual hypervisor Real Mode Access */
-    if (!relocation && (msr_hv || cpu->vhyp)) {
-        /* In real mode top 4 effective addr bits (mostly) ignored */
-        raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
-
-        /* In HV mode, add HRMOR if top EA bit is clear */
-        if (msr_hv || !env->has_hv_mode) {
-            if (!(eaddr >> 63)) {
-                raddr |= env->spr[SPR_HRMOR];
-           }
-        }
-        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
-                     TARGET_PAGE_SIZE);
-        return 0;
-    }
-
-    /*
-     * Check UPRT (we avoid the check in real mode to deal with
-     * transitional states during kexec.
-     */
-    if (!ppc64_use_proc_tbl(cpu)) {
-        qemu_log_mask(LOG_GUEST_ERROR,
-                      "LPCR:UPRT not set in radix mode ! LPCR="
-                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
-    }
-
     /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
-    if (ppc_radix64_xlate(cpu, eaddr, access_type, relocation, &raddr,
+    if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
                           &page_size, &prot, true)) {
         return 1;
     }
@@ -607,18 +605,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
 
 hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
 {
-    CPUPPCState *env = &cpu->env;
     int psize, prot;
     hwaddr raddr;
 
-    /* Handle Real Mode */
-    if ((msr_dr == 0) && (msr_hv || cpu->vhyp)) {
-        /* In real mode top 4 effective addr bits (mostly) ignored */
-        return eaddr & 0x0FFFFFFFFFFFFFFFULL;
-    }
-
-    if (ppc_radix64_xlate(cpu, eaddr, 0, msr_dr, &raddr, &psize,
-                          &prot, false)) {
+    if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
+                          &psize, &prot, false)) {
         return -1;
     }
 
-- 
2.17.1



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

* [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (2 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  3:31   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate Bruno Larsen (billionai)
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Instead of returning non-zero for failure, return true for success.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-radix64.c | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index dd5ae69052..2d5f0850c9 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -463,10 +463,10 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
  *              | = On        | Process Scoped |    Scoped     |
  *              +-------------+----------------+---------------+
  */
-static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
-                             MMUAccessType access_type,
-                             hwaddr *raddr, int *psizep, int *protp,
-                             bool guest_visible)
+static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
+                              MMUAccessType access_type,
+                              hwaddr *raddr, int *psizep, int *protp,
+                              bool guest_visible)
 {
     CPUPPCState *env = &cpu->env;
     uint64_t lpid, pid;
@@ -492,7 +492,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
         }
         *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
         *psizep = TARGET_PAGE_BITS;
-        return 0;
+        return true;
     }
 
     /*
@@ -510,7 +510,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
         if (guest_visible) {
             ppc_radix64_raise_segi(cpu, access_type, eaddr);
         }
-        return 1;
+        return false;
     }
 
     /* Get Process Table */
@@ -523,13 +523,13 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
             if (guest_visible) {
                 ppc_radix64_raise_si(cpu, access_type, eaddr, DSISR_NOPTE);
             }
-            return 1;
+            return false;
         }
         if (!validate_pate(cpu, lpid, &pate)) {
             if (guest_visible) {
                 ppc_radix64_raise_si(cpu, access_type, eaddr, DSISR_R_BADCONFIG);
             }
-            return 1;
+            return false;
         }
     }
 
@@ -549,7 +549,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
                                                    pate, &g_raddr, &prot,
                                                    &psize, guest_visible);
         if (ret) {
-            return ret;
+            return false;
         }
         *psizep = MIN(*psizep, psize);
         *protp &= prot;
@@ -573,7 +573,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
                                                      &prot, &psize, false,
                                                      guest_visible);
             if (ret) {
-                return ret;
+                return false;
             }
             *psizep = MIN(*psizep, psize);
             *protp &= prot;
@@ -582,7 +582,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
         }
     }
 
-    return 0;
+    return true;
 }
 
 int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
@@ -593,8 +593,8 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
     hwaddr raddr;
 
     /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
-    if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
-                          &page_size, &prot, true)) {
+    if (!ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
+                           &page_size, &prot, true)) {
         return 1;
     }
 
@@ -608,8 +608,8 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
     int psize, prot;
     hwaddr raddr;
 
-    if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
-                          &psize, &prot, false)) {
+    if (!ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
+                           &psize, &prot, false)) {
         return -1;
     }
 
-- 
2.17.1



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

* [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (3 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  5:55   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 06/10] target/ppc: Split out ppc_hash32_xlate Bruno Larsen (billionai)
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Mirror the interface of ppc_radix64_xlate, putting all of
the logic for hash64 translation into a single function.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-hash64.c | 125 +++++++++++++++++++---------------------
 1 file changed, 59 insertions(+), 66 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index 2febd369b1..c6b167b4dc 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -873,8 +873,10 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
     return -1;
 }
 
-int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                MMUAccessType access_type, int mmu_idx)
+static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
+                             MMUAccessType access_type,
+                             hwaddr *raddrp, int *psizep, int *protp,
+                             bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -918,9 +920,11 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
             slb = &vrma_slbe;
             if (build_vrma_slbe(cpu, slb) != 0) {
                 /* Invalid VRMA setup, machine check */
-                cs->exception_index = POWERPC_EXCP_MCHECK;
-                env->error_code = 0;
-                return 1;
+                if (guest_visible) {
+                    cs->exception_index = POWERPC_EXCP_MCHECK;
+                    env->error_code = 0;
+                }
+                return false;
             }
 
             goto skip_slb_search;
@@ -929,6 +933,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
 
             /* Emulated old-style RMO mode, bounds check against RMLS */
             if (raddr >= limit) {
+                if (!guest_visible) {
+                    return false;
+                }
                 switch (access_type) {
                 case MMU_INST_FETCH:
                     ppc_hash64_set_isi(cs, SRR1_PROTFAULT);
@@ -943,15 +950,16 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
                 default:
                     g_assert_not_reached();
                 }
-                return 1;
+                return false;
             }
 
             raddr |= env->spr[SPR_RMOR];
         }
-        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
-                     TARGET_PAGE_SIZE);
-        return 0;
+
+        *raddrp = raddr;
+        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+        *psizep = TARGET_PAGE_BITS;
+        return true;
     }
 
     /* 2. Translation is on, so look up the SLB */
@@ -964,6 +972,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
             exit(1);
         }
         /* Segment still not found, generate the appropriate interrupt */
+        if (!guest_visible) {
+            return false;
+        }
         switch (access_type) {
         case MMU_INST_FETCH:
             cs->exception_index = POWERPC_EXCP_ISEG;
@@ -978,20 +989,25 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
         default:
             g_assert_not_reached();
         }
-        return 1;
+        return false;
     }
 
-skip_slb_search:
+ skip_slb_search:
 
     /* 3. Check for segment level no-execute violation */
     if (access_type == MMU_INST_FETCH && (slb->vsid & SLB_VSID_N)) {
-        ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD);
-        return 1;
+        if (guest_visible) {
+            ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD);
+        }
+        return false;
     }
 
     /* 4. Locate the PTE in the hash table */
     ptex = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
     if (ptex == -1) {
+        if (!guest_visible) {
+            return false;
+        }
         switch (access_type) {
         case MMU_INST_FETCH:
             ppc_hash64_set_isi(cs, SRR1_NOPTE);
@@ -1005,7 +1021,7 @@ skip_slb_search:
         default:
             g_assert_not_reached();
         }
-        return 1;
+        return false;
     }
     qemu_log_mask(CPU_LOG_MMU,
                   "found PTE at index %08" HWADDR_PRIx "\n", ptex);
@@ -1021,6 +1037,9 @@ skip_slb_search:
     if (need_prot & ~prot) {
         /* Access right violation */
         qemu_log_mask(CPU_LOG_MMU, "PTE access rejected\n");
+        if (!guest_visible) {
+            return false;
+        }
         if (access_type == MMU_INST_FETCH) {
             int srr1 = 0;
             if (PAGE_EXEC & ~exec_prot) {
@@ -1045,7 +1064,7 @@ skip_slb_search:
             }
             ppc_hash64_set_dsi(cs, eaddr, dsisr);
         }
-        return 1;
+        return false;
     }
 
     qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n");
@@ -1069,66 +1088,40 @@ skip_slb_search:
 
     /* 7. Determine the real address from the PTE */
 
-    raddr = deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, eaddr);
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, 1ULL << apshift);
-
-    return 0;
+    *raddrp = deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, eaddr);
+    *protp = prot;
+    *psizep = apshift;
+    return true;
 }
 
-hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
+int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                MMUAccessType access_type, int mmu_idx)
 {
-    CPUPPCState *env = &cpu->env;
-    ppc_slb_t vrma_slbe;
-    ppc_slb_t *slb;
-    hwaddr ptex, raddr;
-    ppc_hash_pte64_t pte;
-    unsigned apshift;
+    CPUState *cs = CPU(cpu);
+    int page_size, prot;
+    hwaddr raddr;
 
-    /* Handle real mode */
-    if (msr_dr == 0) {
-        /* In real mode the top 4 effective address bits are ignored */
-        raddr = addr & 0x0FFFFFFFFFFFFFFFULL;
+    if (!ppc_hash64_xlate(cpu, eaddr, access_type, &raddr,
+                          &page_size, &prot, true)) {
+        return 1;
+    }
 
-        if (cpu->vhyp) {
-            /*
-             * In virtual hypervisor mode, there's nothing to do:
-             *   EA == GPA == qemu guest address
-             */
-            return raddr;
-        } else if ((msr_hv || !env->has_hv_mode) && !(addr >> 63)) {
-            /* In HV mode, add HRMOR if top EA bit is clear */
-            return raddr | env->spr[SPR_HRMOR];
-        } else if (ppc_hash64_use_vrma(env)) {
-            /* Emulated VRMA mode */
-            slb = &vrma_slbe;
-            if (build_vrma_slbe(cpu, slb) != 0) {
-                return -1;
-            }
-        } else {
-            target_ulong limit = rmls_limit(cpu);
+    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
+                 prot, mmu_idx, 1UL << page_size);
+    return 0;
+}
 
-            /* Emulated old-style RMO mode, bounds check against RMLS */
-            if (raddr >= limit) {
-                return -1;
-            }
-            return raddr | env->spr[SPR_RMOR];
-        }
-    } else {
-        slb = slb_lookup(cpu, addr);
-        if (!slb) {
-            return -1;
-        }
-    }
+hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
+{
+    int psize, prot;
+    hwaddr raddr;
 
-    ptex = ppc_hash64_htab_lookup(cpu, slb, addr, &pte, &apshift);
-    if (ptex == -1) {
+    if (!ppc_hash64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
+                          &psize, &prot, false)) {
         return -1;
     }
 
-    return deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, addr)
-        & TARGET_PAGE_MASK;
+    return raddr & TARGET_PAGE_MASK;
 }
 
 void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
-- 
2.17.1



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

* [PATCH v2 06/10] target/ppc: Split out ppc_hash32_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (4 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate Bruno Larsen (billionai)
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Mirror the interface of ppc_radix64_xlate, putting all of
the logic for hash32 translation into a single entry point.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-hash32.c | 224 ++++++++++++++++++++--------------------
 1 file changed, 113 insertions(+), 111 deletions(-)

diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index 8f19b43e47..ad22372c07 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -218,10 +218,11 @@ static hwaddr ppc_hash32_bat_lookup(PowerPCCPU *cpu, target_ulong ea,
     return -1;
 }
 
-static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
-                                   target_ulong eaddr,
-                                   MMUAccessType access_type,
-                                   hwaddr *raddr, int *prot)
+static bool ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
+                                    target_ulong eaddr,
+                                    MMUAccessType access_type,
+                                    hwaddr *raddr, int *prot,
+                                    bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -238,17 +239,23 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
          */
         *raddr = ((sr & 0xF) << 28) | (eaddr & 0x0FFFFFFF);
         *prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
-        return 0;
+        return true;
     }
 
     if (access_type == MMU_INST_FETCH) {
         /* No code fetch is allowed in direct-store areas */
-        cs->exception_index = POWERPC_EXCP_ISI;
-        env->error_code = 0x10000000;
-        return 1;
+        if (guest_visible) {
+            cs->exception_index = POWERPC_EXCP_ISI;
+            env->error_code = 0x10000000;
+        }
+        return false;
     }
 
-    switch (env->access_type) {
+    /*
+     * From ppc_cpu_get_phys_page_debug, env->access_type is not set.
+     * Assume ACCESS_INT for that case.
+     */
+    switch (guest_visible ? env->access_type : ACCESS_INT) {
     case ACCESS_INT:
         /* Integer load/store : only access allowed */
         break;
@@ -257,7 +264,7 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
         cs->exception_index = POWERPC_EXCP_ALIGN;
         env->error_code = POWERPC_EXCP_ALIGN_FP;
         env->spr[SPR_DAR] = eaddr;
-        return 1;
+        return false;
     case ACCESS_RES:
         /* lwarx, ldarx or srwcx. */
         env->error_code = 0;
@@ -267,7 +274,7 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
         } else {
             env->spr[SPR_DSISR] = 0x04000000;
         }
-        return 1;
+        return false;
     case ACCESS_CACHE:
         /*
          * dcba, dcbt, dcbtst, dcbf, dcbi, dcbst, dcbz, or icbi
@@ -276,7 +283,7 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
          * no-op, it's quite easy :-)
          */
         *raddr = eaddr;
-        return 0;
+        return true;
     case ACCESS_EXT:
         /* eciwx or ecowx */
         cs->exception_index = POWERPC_EXCP_DSI;
@@ -287,16 +294,18 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
         } else {
             env->spr[SPR_DSISR] = 0x04100000;
         }
-        return 1;
+        return false;
     default:
-        cpu_abort(cs, "ERROR: instruction should not need "
-                 "address translation\n");
+        cpu_abort(cs, "ERROR: insn should not need address translation\n");
     }
-    if ((access_type == MMU_DATA_STORE || key != 1) &&
-        (access_type == MMU_DATA_LOAD || key != 0)) {
+
+    *prot = key ? PAGE_READ | PAGE_WRITE : PAGE_READ;
+    if (*prot & prot_for_access_type(access_type)) {
         *raddr = eaddr;
-        return 0;
-    } else {
+        return true;
+    }
+
+    if (guest_visible) {
         cs->exception_index = POWERPC_EXCP_DSI;
         env->error_code = 0;
         env->spr[SPR_DAR] = eaddr;
@@ -305,8 +314,8 @@ static int ppc_hash32_direct_store(PowerPCCPU *cpu, target_ulong sr,
         } else {
             env->spr[SPR_DSISR] = 0x08000000;
         }
-        return 1;
     }
+    return false;
 }
 
 hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash)
@@ -415,8 +424,10 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
     return (rpn & ~mask) | (eaddr & mask);
 }
 
-int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                MMUAccessType access_type, int mmu_idx)
+static bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr,
+                             MMUAccessType access_type,
+                             hwaddr *raddrp, int *psizep, int *protp,
+                             bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -427,43 +438,43 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
     int need_prot;
     hwaddr raddr;
 
-    need_prot = prot_for_access_type(access_type);
+    /* There are no hash32 large pages. */
+    *psizep = TARGET_PAGE_BITS;
 
     /* 1. Handle real mode accesses */
     if (access_type == MMU_INST_FETCH ? !msr_ir : !msr_dr) {
         /* Translation is off */
-        raddr = eaddr;
-        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
-                     TARGET_PAGE_SIZE);
-        return 0;
+        *raddrp = eaddr;
+        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
+        return true;
     }
 
+    need_prot = prot_for_access_type(access_type);
+
     /* 2. Check Block Address Translation entries (BATs) */
     if (env->nb_BATs != 0) {
-        raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, &prot);
+        raddr = ppc_hash32_bat_lookup(cpu, eaddr, access_type, protp);
         if (raddr != -1) {
-            if (need_prot & ~prot) {
-                if (access_type == MMU_INST_FETCH) {
-                    cs->exception_index = POWERPC_EXCP_ISI;
-                    env->error_code = 0x08000000;
-                } else {
-                    cs->exception_index = POWERPC_EXCP_DSI;
-                    env->error_code = 0;
-                    env->spr[SPR_DAR] = eaddr;
-                    if (access_type == MMU_DATA_STORE) {
-                        env->spr[SPR_DSISR] = 0x0a000000;
+            if (need_prot & ~*protp) {
+                if (guest_visible) {
+                    if (access_type == MMU_INST_FETCH) {
+                        cs->exception_index = POWERPC_EXCP_ISI;
+                        env->error_code = 0x08000000;
                     } else {
-                        env->spr[SPR_DSISR] = 0x08000000;
+                        cs->exception_index = POWERPC_EXCP_DSI;
+                        env->error_code = 0;
+                        env->spr[SPR_DAR] = eaddr;
+                        if (access_type == MMU_DATA_STORE) {
+                            env->spr[SPR_DSISR] = 0x0a000000;
+                        } else {
+                            env->spr[SPR_DSISR] = 0x08000000;
+                        }
                     }
                 }
-                return 1;
+                return false;
             }
-
-            tlb_set_page(cs, eaddr & TARGET_PAGE_MASK,
-                         raddr & TARGET_PAGE_MASK, prot, mmu_idx,
-                         TARGET_PAGE_SIZE);
-            return 0;
+            *raddrp = raddr;
+            return true;
         }
     }
 
@@ -472,42 +483,38 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
 
     /* 4. Handle direct store segments */
     if (sr & SR32_T) {
-        if (ppc_hash32_direct_store(cpu, sr, eaddr, access_type,
-                                    &raddr, &prot) == 0) {
-            tlb_set_page(cs, eaddr & TARGET_PAGE_MASK,
-                         raddr & TARGET_PAGE_MASK, prot, mmu_idx,
-                         TARGET_PAGE_SIZE);
-            return 0;
-        } else {
-            return 1;
-        }
+        return ppc_hash32_direct_store(cpu, sr, eaddr, access_type,
+                                       raddrp, protp, guest_visible);
     }
 
     /* 5. Check for segment level no-execute violation */
     if (access_type == MMU_INST_FETCH && (sr & SR32_NX)) {
-        cs->exception_index = POWERPC_EXCP_ISI;
-        env->error_code = 0x10000000;
-        return 1;
+        if (guest_visible) {
+            cs->exception_index = POWERPC_EXCP_ISI;
+            env->error_code = 0x10000000;
+        }
+        return false;
     }
 
     /* 6. Locate the PTE in the hash table */
     pte_offset = ppc_hash32_htab_lookup(cpu, sr, eaddr, &pte);
     if (pte_offset == -1) {
-        if (access_type == MMU_INST_FETCH) {
-            cs->exception_index = POWERPC_EXCP_ISI;
-            env->error_code = 0x40000000;
-        } else {
-            cs->exception_index = POWERPC_EXCP_DSI;
-            env->error_code = 0;
-            env->spr[SPR_DAR] = eaddr;
-            if (access_type == MMU_DATA_STORE) {
-                env->spr[SPR_DSISR] = 0x42000000;
+        if (guest_visible) {
+            if (access_type == MMU_INST_FETCH) {
+                cs->exception_index = POWERPC_EXCP_ISI;
+                env->error_code = 0x40000000;
             } else {
-                env->spr[SPR_DSISR] = 0x40000000;
+                cs->exception_index = POWERPC_EXCP_DSI;
+                env->error_code = 0;
+                env->spr[SPR_DAR] = eaddr;
+                if (access_type == MMU_DATA_STORE) {
+                    env->spr[SPR_DSISR] = 0x42000000;
+                } else {
+                    env->spr[SPR_DSISR] = 0x40000000;
+                }
             }
         }
-
-        return 1;
+        return false;
     }
     qemu_log_mask(CPU_LOG_MMU,
                 "found PTE at offset %08" HWADDR_PRIx "\n", pte_offset);
@@ -519,20 +526,22 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
     if (need_prot & ~prot) {
         /* Access right violation */
         qemu_log_mask(CPU_LOG_MMU, "PTE access rejected\n");
-        if (access_type == MMU_INST_FETCH) {
-            cs->exception_index = POWERPC_EXCP_ISI;
-            env->error_code = 0x08000000;
-        } else {
-            cs->exception_index = POWERPC_EXCP_DSI;
-            env->error_code = 0;
-            env->spr[SPR_DAR] = eaddr;
-            if (access_type == MMU_DATA_STORE) {
-                env->spr[SPR_DSISR] = 0x0a000000;
+        if (guest_visible) {
+            if (access_type == MMU_INST_FETCH) {
+                cs->exception_index = POWERPC_EXCP_ISI;
+                env->error_code = 0x08000000;
             } else {
-                env->spr[SPR_DSISR] = 0x08000000;
+                cs->exception_index = POWERPC_EXCP_DSI;
+                env->error_code = 0;
+                env->spr[SPR_DAR] = eaddr;
+                if (access_type == MMU_DATA_STORE) {
+                    env->spr[SPR_DSISR] = 0x0a000000;
+                } else {
+                    env->spr[SPR_DSISR] = 0x08000000;
+                }
             }
         }
-        return 1;
+        return false;
     }
 
     qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n");
@@ -556,45 +565,38 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
 
     /* 9. Determine the real address from the PTE */
 
-    raddr = ppc_hash32_pte_raddr(sr, pte, eaddr);
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, TARGET_PAGE_SIZE);
-
-    return 0;
+    *raddrp = ppc_hash32_pte_raddr(sr, pte, eaddr);
+    *protp = prot;
+    return true;
 }
 
-hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
+int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                MMUAccessType access_type, int mmu_idx)
 {
-    CPUPPCState *env = &cpu->env;
-    target_ulong sr;
-    hwaddr pte_offset;
-    ppc_hash_pte32_t pte;
-    int prot;
-
-    if (msr_dr == 0) {
-        /* Translation is off */
-        return eaddr;
-    }
+    CPUState *cs = CPU(cpu);
+    int page_size, prot;
+    hwaddr raddr;
 
-    if (env->nb_BATs != 0) {
-        hwaddr raddr = ppc_hash32_bat_lookup(cpu, eaddr, 0, &prot);
-        if (raddr != -1) {
-            return raddr;
-        }
+    /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
+    if (!ppc_hash32_xlate(cpu, eaddr, access_type, &raddr,
+                           &page_size, &prot, true)) {
+        return 1;
     }
 
-    sr = env->sr[eaddr >> 28];
+    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
+                 prot, mmu_idx, 1UL << page_size);
+    return 0;
+}
 
-    if (sr & SR32_T) {
-        /* FIXME: Add suitable debug support for Direct Store segments */
-        return -1;
-    }
+hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
+{
+    int psize, prot;
+    hwaddr raddr;
 
-    pte_offset = ppc_hash32_htab_lookup(cpu, sr, eaddr, &pte);
-    if (pte_offset == -1) {
+    if (!ppc_hash32_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
+                           &psize, &prot, false)) {
         return -1;
     }
 
-    return ppc_hash32_pte_raddr(sr, pte, eaddr) & TARGET_PAGE_MASK;
+    return raddr & TARGET_PAGE_MASK;
 }
-- 
2.17.1



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

* [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (5 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 06/10] target/ppc: Split out ppc_hash32_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  6:30   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 08/10] target/ppc: Introduce ppc_xlate Bruno Larsen (billionai)
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Mirror the interface of ppc_radix64_xlate (mostly), putting all
of the logic for older mmu translation into a single entry point.
For booke, we need to add mmu_idx to the xlate-style interface.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu_helper.c | 179 +++++++++++++++++++++-------------------
 1 file changed, 96 insertions(+), 83 deletions(-)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index c4b1c93e47..2e92deb105 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -1435,48 +1435,6 @@ static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
 }
 #endif
 
-hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
-{
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-    CPUPPCState *env = &cpu->env;
-    mmu_ctx_t ctx;
-
-    switch (env->mmu_model) {
-#if defined(TARGET_PPC64)
-    case POWERPC_MMU_64B:
-    case POWERPC_MMU_2_03:
-    case POWERPC_MMU_2_06:
-    case POWERPC_MMU_2_07:
-        return ppc_hash64_get_phys_page_debug(cpu, addr);
-    case POWERPC_MMU_3_00:
-        return ppc64_v3_get_phys_page_debug(cpu, addr);
-#endif
-
-    case POWERPC_MMU_32B:
-    case POWERPC_MMU_601:
-        return ppc_hash32_get_phys_page_debug(cpu, addr);
-
-    default:
-        ;
-    }
-
-    if (unlikely(get_physical_address(env, &ctx, addr, MMU_DATA_LOAD,
-                                      ACCESS_INT) != 0)) {
-
-        /*
-         * Some MMUs have separate TLBs for code and data. If we only
-         * try an ACCESS_INT, we may not be able to read instructions
-         * mapped by code TLBs, so we also try a ACCESS_CODE.
-         */
-        if (unlikely(get_physical_address(env, &ctx, addr, MMU_INST_FETCH,
-                                          ACCESS_CODE) != 0)) {
-            return -1;
-        }
-    }
-
-    return ctx.raddr & TARGET_PAGE_MASK;
-}
-
 static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
                                          MMUAccessType access_type, int mmu_idx)
 {
@@ -1532,30 +1490,38 @@ static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
 }
 
 /* Perform address translation */
-static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
-                                    MMUAccessType access_type, int mmu_idx)
+/* TODO: Split this by mmu_model. */
+static bool ppc_jumbo_xlate(PowerPCCPU *cpu, vaddr eaddr,
+                            MMUAccessType access_type,
+                            hwaddr *raddrp, int *psizep, int *protp,
+                            int mmu_idx, bool guest_visible)
 {
-    CPUState *cs = env_cpu(env);
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
     mmu_ctx_t ctx;
     int type;
-    int ret = 0;
+    int ret;
 
     if (access_type == MMU_INST_FETCH) {
         /* code access */
         type = ACCESS_CODE;
-    } else {
+    } else if (guest_visible) {
         /* data access */
         type = env->access_type;
+    } else {
+        type = ACCESS_INT;
     }
-    ret = get_physical_address_wtlb(env, &ctx, address, access_type,
+
+    ret = get_physical_address_wtlb(env, &ctx, eaddr, access_type,
                                     type, mmu_idx);
     if (ret == 0) {
-        tlb_set_page(cs, address & TARGET_PAGE_MASK,
-                     ctx.raddr & TARGET_PAGE_MASK, ctx.prot,
-                     mmu_idx, TARGET_PAGE_SIZE);
-        ret = 0;
-    } else if (ret < 0) {
+        *raddrp = ctx.raddr;
+        *protp = ctx.prot;
+        *psizep = TARGET_PAGE_BITS;
+        return true;
+    }
+
+    if (guest_visible) {
         LOG_MMU_STATE(cs);
         if (type == ACCESS_CODE) {
             switch (ret) {
@@ -1565,7 +1531,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 case POWERPC_MMU_SOFT_6xx:
                     cs->exception_index = POWERPC_EXCP_IFTLB;
                     env->error_code = 1 << 18;
-                    env->spr[SPR_IMISS] = address;
+                    env->spr[SPR_IMISS] = eaddr;
                     env->spr[SPR_ICMP] = 0x80000000 | ctx.ptem;
                     goto tlb_miss;
                 case POWERPC_MMU_SOFT_74xx:
@@ -1575,29 +1541,25 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 case POWERPC_MMU_SOFT_4xx_Z:
                     cs->exception_index = POWERPC_EXCP_ITLB;
                     env->error_code = 0;
-                    env->spr[SPR_40x_DEAR] = address;
+                    env->spr[SPR_40x_DEAR] = eaddr;
                     env->spr[SPR_40x_ESR] = 0x00000000;
                     break;
                 case POWERPC_MMU_BOOKE206:
-                    booke206_update_mas_tlb_miss(env, address, 2, mmu_idx);
+                    booke206_update_mas_tlb_miss(env, eaddr, 2, mmu_idx);
                     /* fall through */
                 case POWERPC_MMU_BOOKE:
                     cs->exception_index = POWERPC_EXCP_ITLB;
                     env->error_code = 0;
-                    env->spr[SPR_BOOKE_DEAR] = address;
+                    env->spr[SPR_BOOKE_DEAR] = eaddr;
                     env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, MMU_DATA_LOAD);
-                    return -1;
+                    break;
                 case POWERPC_MMU_MPC8xx:
-                    /* XXX: TODO */
                     cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
-                    break;
                 case POWERPC_MMU_REAL:
                     cpu_abort(cs, "PowerPC in real mode should never raise "
                               "any MMU exceptions\n");
-                    return -1;
                 default:
                     cpu_abort(cs, "Unknown or invalid MMU model\n");
-                    return -1;
                 }
                 break;
             case -2:
@@ -1634,7 +1596,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                         cs->exception_index = POWERPC_EXCP_DLTLB;
                         env->error_code = 0;
                     }
-                    env->spr[SPR_DMISS] = address;
+                    env->spr[SPR_DMISS] = eaddr;
                     env->spr[SPR_DCMP] = 0x80000000 | ctx.ptem;
                 tlb_miss:
                     env->error_code |= ctx.key << 19;
@@ -1652,7 +1614,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 tlb_miss_74xx:
                     /* Implement LRU algorithm */
                     env->error_code = ctx.key << 19;
-                    env->spr[SPR_TLBMISS] = (address & ~((target_ulong)0x3)) |
+                    env->spr[SPR_TLBMISS] = (eaddr & ~((target_ulong)0x3)) |
                         ((env->last_way + 1) & (env->nb_ways - 1));
                     env->spr[SPR_PTEHI] = 0x80000000 | ctx.ptem;
                     break;
@@ -1660,7 +1622,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 case POWERPC_MMU_SOFT_4xx_Z:
                     cs->exception_index = POWERPC_EXCP_DTLB;
                     env->error_code = 0;
-                    env->spr[SPR_40x_DEAR] = address;
+                    env->spr[SPR_40x_DEAR] = eaddr;
                     if (access_type == MMU_DATA_STORE) {
                         env->spr[SPR_40x_ESR] = 0x00800000;
                     } else {
@@ -1670,23 +1632,20 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 case POWERPC_MMU_MPC8xx:
                     /* XXX: TODO */
                     cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
-                    break;
                 case POWERPC_MMU_BOOKE206:
-                    booke206_update_mas_tlb_miss(env, address, access_type, mmu_idx);
+                    booke206_update_mas_tlb_miss(env, eaddr, access_type, mmu_idx);
                     /* fall through */
                 case POWERPC_MMU_BOOKE:
                     cs->exception_index = POWERPC_EXCP_DTLB;
                     env->error_code = 0;
-                    env->spr[SPR_BOOKE_DEAR] = address;
+                    env->spr[SPR_BOOKE_DEAR] = eaddr;
                     env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
-                    return -1;
+                    break;
                 case POWERPC_MMU_REAL:
                     cpu_abort(cs, "PowerPC in real mode should never raise "
                               "any MMU exceptions\n");
-                    return -1;
                 default:
                     cpu_abort(cs, "Unknown or invalid MMU model\n");
-                    return -1;
                 }
                 break;
             case -2:
@@ -1695,16 +1654,16 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                 env->error_code = 0;
                 if (env->mmu_model == POWERPC_MMU_SOFT_4xx
                     || env->mmu_model == POWERPC_MMU_SOFT_4xx_Z) {
-                    env->spr[SPR_40x_DEAR] = address;
+                    env->spr[SPR_40x_DEAR] = eaddr;
                     if (access_type == MMU_DATA_STORE) {
                         env->spr[SPR_40x_ESR] |= 0x00800000;
                     }
                 } else if ((env->mmu_model == POWERPC_MMU_BOOKE) ||
                            (env->mmu_model == POWERPC_MMU_BOOKE206)) {
-                    env->spr[SPR_BOOKE_DEAR] = address;
+                    env->spr[SPR_BOOKE_DEAR] = eaddr;
                     env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
                 } else {
-                    env->spr[SPR_DAR] = address;
+                    env->spr[SPR_DAR] = eaddr;
                     if (access_type == MMU_DATA_STORE) {
                         env->spr[SPR_DSISR] = 0x0A000000;
                     } else {
@@ -1719,13 +1678,13 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                     /* Floating point load/store */
                     cs->exception_index = POWERPC_EXCP_ALIGN;
                     env->error_code = POWERPC_EXCP_ALIGN_FP;
-                    env->spr[SPR_DAR] = address;
+                    env->spr[SPR_DAR] = eaddr;
                     break;
                 case ACCESS_RES:
                     /* lwarx, ldarx or stwcx. */
                     cs->exception_index = POWERPC_EXCP_DSI;
                     env->error_code = 0;
-                    env->spr[SPR_DAR] = address;
+                    env->spr[SPR_DAR] = eaddr;
                     if (access_type == MMU_DATA_STORE) {
                         env->spr[SPR_DSISR] = 0x06000000;
                     } else {
@@ -1736,7 +1695,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                     /* eciwx or ecowx */
                     cs->exception_index = POWERPC_EXCP_DSI;
                     env->error_code = 0;
-                    env->spr[SPR_DAR] = address;
+                    env->spr[SPR_DAR] = eaddr;
                     if (access_type == MMU_DATA_STORE) {
                         env->spr[SPR_DSISR] = 0x06100000;
                     } else {
@@ -1748,16 +1707,14 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
                     cs->exception_index = POWERPC_EXCP_PROGRAM;
                     env->error_code =
                         POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL;
-                    env->spr[SPR_DAR] = address;
+                    env->spr[SPR_DAR] = eaddr;
                     break;
                 }
                 break;
             }
         }
-        ret = 1;
     }
-
-    return ret;
+    return false;
 }
 
 #ifdef CONFIG_TCG
@@ -2942,6 +2899,62 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
 
 /*****************************************************************************/
 
+static int cpu_ppc_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
+                                    MMUAccessType access_type, int mmu_idx)
+{
+    CPUState *cs = CPU(cpu);
+    int page_size, prot;
+    hwaddr raddr;
+
+    if (!ppc_jumbo_xlate(cpu, eaddr, access_type, &raddr,
+                         &page_size, &prot, mmu_idx, true)) {
+        return 1;
+    }
+
+    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
+                 prot, mmu_idx, 1UL << page_size);
+    return 0;
+}
+
+hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    CPUPPCState *env = &cpu->env;
+    hwaddr raddr;
+    int s, p;
+
+    switch (env->mmu_model) {
+#if defined(TARGET_PPC64)
+    case POWERPC_MMU_64B:
+    case POWERPC_MMU_2_03:
+    case POWERPC_MMU_2_06:
+    case POWERPC_MMU_2_07:
+        return ppc_hash64_get_phys_page_debug(cpu, addr);
+    case POWERPC_MMU_3_00:
+        return ppc64_v3_get_phys_page_debug(cpu, addr);
+#endif
+
+    case POWERPC_MMU_32B:
+    case POWERPC_MMU_601:
+        return ppc_hash32_get_phys_page_debug(cpu, addr);
+
+    default:
+        ;
+    }
+
+    /*
+     * Some MMUs have separate TLBs for code and data. If we only
+     * try an MMU_DATA_LOAD, we may not be able to read instructions
+     * mapped by code TLBs, so we also try a MMU_INST_FETCH.
+     */
+    if (ppc_jumbo_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
+        ppc_jumbo_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
+        return raddr & TARGET_PAGE_MASK;
+    }
+    return -1;
+}
+
+
 bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr)
@@ -2969,7 +2982,7 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
         break;
 
     default:
-        ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
+        ret = cpu_ppc_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
         break;
     }
     if (unlikely(ret != 0)) {
-- 
2.17.1



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

* [PATCH v2 08/10] target/ppc: Introduce ppc_xlate
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (6 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  6:34   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG Bruno Larsen (billionai)
  2021-06-21 12:51 ` [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

Create one common dispatch for all of the ppc_*_xlate functions.
Use ppc64_v3_radix to directly dispatch between ppc_radix64_xlate
and ppc_hash64_xlate.

Remove the separate *_handle_mmu_fault and *_get_phys_page_debug
functions, using common code for ppc_cpu_tlb_fill and
ppc_cpu_get_phys_page_debug.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu-book3s-v3.c |  19 -------
 target/ppc/mmu-book3s-v3.h |   5 --
 target/ppc/mmu-hash32.c    |  38 ++------------
 target/ppc/mmu-hash32.h    |   6 +--
 target/ppc/mmu-hash64.c    |  37 ++------------
 target/ppc/mmu-hash64.h    |   6 +--
 target/ppc/mmu-radix64.c   |  38 ++------------
 target/ppc/mmu-radix64.h   |   6 +--
 target/ppc/mmu_helper.c    | 100 ++++++++++++++-----------------------
 9 files changed, 55 insertions(+), 200 deletions(-)

diff --git a/target/ppc/mmu-book3s-v3.c b/target/ppc/mmu-book3s-v3.c
index c78fd8dc0e..f4985bae78 100644
--- a/target/ppc/mmu-book3s-v3.c
+++ b/target/ppc/mmu-book3s-v3.c
@@ -23,25 +23,6 @@
 #include "mmu-book3s-v3.h"
 #include "mmu-radix64.h"
 
-int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
-                              int mmu_idx)
-{
-    if (ppc64_v3_radix(cpu)) { /* Guest uses radix */
-        return ppc_radix64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
-    } else { /* Guest uses hash */
-        return ppc_hash64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
-    }
-}
-
-hwaddr ppc64_v3_get_phys_page_debug(PowerPCCPU *cpu, vaddr eaddr)
-{
-    if (ppc64_v3_radix(cpu)) {
-        return ppc_radix64_get_phys_page_debug(cpu, eaddr);
-    } else {
-        return ppc_hash64_get_phys_page_debug(cpu, eaddr);
-    }
-}
-
 bool ppc64_v3_get_pate(PowerPCCPU *cpu, target_ulong lpid, ppc_v3_pate_t *entry)
 {
     uint64_t patb = cpu->env.spr[SPR_PTCR] & PTCR_PATB;
diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h
index 7b89be54b8..a1326df969 100644
--- a/target/ppc/mmu-book3s-v3.h
+++ b/target/ppc/mmu-book3s-v3.h
@@ -67,11 +67,6 @@ static inline bool ppc64_v3_radix(PowerPCCPU *cpu)
     return !!(cpu->env.spr[SPR_LPCR] & LPCR_HR);
 }
 
-hwaddr ppc64_v3_get_phys_page_debug(PowerPCCPU *cpu, vaddr eaddr);
-
-int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
-                              int mmu_idx);
-
 static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
 {
     uint64_t base;
diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
index ad22372c07..6a07c345e4 100644
--- a/target/ppc/mmu-hash32.c
+++ b/target/ppc/mmu-hash32.c
@@ -424,10 +424,9 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
     return (rpn & ~mask) | (eaddr & mask);
 }
 
-static bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr,
-                             MMUAccessType access_type,
-                             hwaddr *raddrp, int *psizep, int *protp,
-                             bool guest_visible)
+bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                      hwaddr *raddrp, int *psizep, int *protp,
+                      bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -569,34 +568,3 @@ static bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr,
     *protp = prot;
     return true;
 }
-
-int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                MMUAccessType access_type, int mmu_idx)
-{
-    CPUState *cs = CPU(cpu);
-    int page_size, prot;
-    hwaddr raddr;
-
-    /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
-    if (!ppc_hash32_xlate(cpu, eaddr, access_type, &raddr,
-                           &page_size, &prot, true)) {
-        return 1;
-    }
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, 1UL << page_size);
-    return 0;
-}
-
-hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
-{
-    int psize, prot;
-    hwaddr raddr;
-
-    if (!ppc_hash32_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
-                           &psize, &prot, false)) {
-        return -1;
-    }
-
-    return raddr & TARGET_PAGE_MASK;
-}
diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
index 30e35718a7..8694eccabd 100644
--- a/target/ppc/mmu-hash32.h
+++ b/target/ppc/mmu-hash32.h
@@ -4,9 +4,9 @@
 #ifndef CONFIG_USER_ONLY
 
 hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
-hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
-int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
-                                MMUAccessType access_type, int mmu_idx);
+bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                      hwaddr *raddrp, int *psizep, int *protp,
+                      bool guest_visible);
 
 /*
  * Segment register definitions
diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index c6b167b4dc..c1b98a97e9 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -873,10 +873,9 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
     return -1;
 }
 
-static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
-                             MMUAccessType access_type,
-                             hwaddr *raddrp, int *psizep, int *protp,
-                             bool guest_visible)
+bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                      hwaddr *raddrp, int *psizep, int *protp,
+                      bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -1094,36 +1093,6 @@ static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
     return true;
 }
 
-int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                MMUAccessType access_type, int mmu_idx)
-{
-    CPUState *cs = CPU(cpu);
-    int page_size, prot;
-    hwaddr raddr;
-
-    if (!ppc_hash64_xlate(cpu, eaddr, access_type, &raddr,
-                          &page_size, &prot, true)) {
-        return 1;
-    }
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, 1UL << page_size);
-    return 0;
-}
-
-hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
-{
-    int psize, prot;
-    hwaddr raddr;
-
-    if (!ppc_hash64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
-                          &psize, &prot, false)) {
-        return -1;
-    }
-
-    return raddr & TARGET_PAGE_MASK;
-}
-
 void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
                                target_ulong pte0, target_ulong pte1)
 {
diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
index 3e8a8eec1f..9f338e1fe9 100644
--- a/target/ppc/mmu-hash64.h
+++ b/target/ppc/mmu-hash64.h
@@ -7,9 +7,9 @@
 void dump_slb(PowerPCCPU *cpu);
 int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
                   target_ulong esid, target_ulong vsid);
-hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
-int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
-                                MMUAccessType access_type, int mmu_idx);
+bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                      hwaddr *raddrp, int *psizep, int *protp,
+                      bool guest_visible);
 void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
                                target_ulong pte_index,
                                target_ulong pte0, target_ulong pte1);
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index 2d5f0850c9..cbd404bfa4 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -463,10 +463,9 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
  *              | = On        | Process Scoped |    Scoped     |
  *              +-------------+----------------+---------------+
  */
-static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
-                              MMUAccessType access_type,
-                              hwaddr *raddr, int *psizep, int *protp,
-                              bool guest_visible)
+bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                       hwaddr *raddr, int *psizep, int *protp,
+                       bool guest_visible)
 {
     CPUPPCState *env = &cpu->env;
     uint64_t lpid, pid;
@@ -584,34 +583,3 @@ static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
 
     return true;
 }
-
-int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                 MMUAccessType access_type, int mmu_idx)
-{
-    CPUState *cs = CPU(cpu);
-    int page_size, prot;
-    hwaddr raddr;
-
-    /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
-    if (!ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
-                           &page_size, &prot, true)) {
-        return 1;
-    }
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, 1UL << page_size);
-    return 0;
-}
-
-hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
-{
-    int psize, prot;
-    hwaddr raddr;
-
-    if (!ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
-                           &psize, &prot, false)) {
-        return -1;
-    }
-
-    return raddr & TARGET_PAGE_MASK;
-}
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index 94bd72cb38..6b13b89b64 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -44,9 +44,9 @@
 
 #ifdef TARGET_PPC64
 
-int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                 MMUAccessType access_type, int mmu_idx);
-hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
+bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                       hwaddr *raddr, int *psizep, int *protp,
+                       bool guest_visible);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
 {
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index 2e92deb105..a0e4e027d3 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2899,98 +2899,72 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
 
 /*****************************************************************************/
 
-static int cpu_ppc_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
-                                    MMUAccessType access_type, int mmu_idx)
+static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
+                      hwaddr *raddrp, int *psizep, int *protp,
+                      int mmu_idx, bool guest_visible)
 {
-    CPUState *cs = CPU(cpu);
-    int page_size, prot;
-    hwaddr raddr;
-
-    if (!ppc_jumbo_xlate(cpu, eaddr, access_type, &raddr,
-                         &page_size, &prot, mmu_idx, true)) {
-        return 1;
-    }
-
-    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
-                 prot, mmu_idx, 1UL << page_size);
-    return 0;
-}
-
-hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
-{
-    PowerPCCPU *cpu = POWERPC_CPU(cs);
-    CPUPPCState *env = &cpu->env;
-    hwaddr raddr;
-    int s, p;
-
-    switch (env->mmu_model) {
+    switch (cpu->env.mmu_model) {
 #if defined(TARGET_PPC64)
+    case POWERPC_MMU_3_00:
+        if (ppc64_v3_radix(cpu)) {
+            return ppc_radix64_xlate(cpu, eaddr, access_type,
+                                     raddrp, psizep, protp, guest_visible);
+        }
+        /* fall through */
     case POWERPC_MMU_64B:
     case POWERPC_MMU_2_03:
     case POWERPC_MMU_2_06:
     case POWERPC_MMU_2_07:
-        return ppc_hash64_get_phys_page_debug(cpu, addr);
-    case POWERPC_MMU_3_00:
-        return ppc64_v3_get_phys_page_debug(cpu, addr);
+        return ppc_hash64_xlate(cpu, eaddr, access_type,
+                                raddrp, psizep, protp, guest_visible);
 #endif
 
     case POWERPC_MMU_32B:
     case POWERPC_MMU_601:
-        return ppc_hash32_get_phys_page_debug(cpu, addr);
+        return ppc_hash32_xlate(cpu, eaddr, access_type,
+                                raddrp, psizep, protp, guest_visible);
 
     default:
-        ;
+        return ppc_jumbo_xlate(cpu, eaddr, access_type, raddrp,
+                               psizep, protp, mmu_idx, guest_visible);
     }
+}
+
+hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
+{
+    PowerPCCPU *cpu = POWERPC_CPU(cs);
+    hwaddr raddr;
+    int s, p;
 
     /*
      * Some MMUs have separate TLBs for code and data. If we only
      * try an MMU_DATA_LOAD, we may not be able to read instructions
      * mapped by code TLBs, so we also try a MMU_INST_FETCH.
      */
-    if (ppc_jumbo_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
-        ppc_jumbo_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
+    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
+        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
         return raddr & TARGET_PAGE_MASK;
     }
     return -1;
 }
 
-
-bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
+bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
-    CPUPPCState *env = &cpu->env;
-    int ret;
-
-    switch (env->mmu_model) {
-#if defined(TARGET_PPC64)
-    case POWERPC_MMU_64B:
-    case POWERPC_MMU_2_03:
-    case POWERPC_MMU_2_06:
-    case POWERPC_MMU_2_07:
-        ret = ppc_hash64_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
-        break;
-    case POWERPC_MMU_3_00:
-        ret = ppc64_v3_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
-        break;
-#endif
-
-    case POWERPC_MMU_32B:
-    case POWERPC_MMU_601:
-        ret = ppc_hash32_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
-        break;
+    hwaddr raddr;
+    int page_size, prot;
 
-    default:
-        ret = cpu_ppc_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
-        break;
+    if (ppc_xlate(cpu, eaddr, access_type, &raddr,
+                  &page_size, &prot, mmu_idx, !probe)) {
+        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
+                     prot, mmu_idx, 1UL << page_size);
+        return true;
     }
-    if (unlikely(ret != 0)) {
-        if (probe) {
-            return false;
-        }
-        raise_exception_err_ra(env, cs->exception_index, env->error_code,
-                               retaddr);
+    if (probe) {
+        return false;
     }
-    return true;
+    raise_exception_err_ra(&cpu->env, cs->exception_index,
+                           cpu->env.error_code, retaddr);
 }
-- 
2.17.1



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

* [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (7 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 08/10] target/ppc: Introduce ppc_xlate Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  6:35   ` David Gibson
  2021-06-21 12:51 ` [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, Richard Henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

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

This function is used by TCGCPUOps, and is thus TCG specific.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/mmu_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index a0e4e027d3..ba1952c77d 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2948,6 +2948,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
     return -1;
 }
 
+#ifdef CONFIG_TCG
 bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
                       MMUAccessType access_type, int mmu_idx,
                       bool probe, uintptr_t retaddr)
@@ -2968,3 +2969,4 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
     raise_exception_err_ra(&cpu->env, cs->exception_index,
                            cpu->env.error_code, retaddr);
 }
+#endif
-- 
2.17.1



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

* [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus
  2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
                   ` (8 preceding siblings ...)
  2021-06-21 12:51 ` [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG Bruno Larsen (billionai)
@ 2021-06-21 12:51 ` Bruno Larsen (billionai)
  2021-06-24  6:48   ` David Gibson
  9 siblings, 1 reply; 22+ messages in thread
From: Bruno Larsen (billionai) @ 2021-06-21 12:51 UTC (permalink / raw)
  To: qemu-devel
  Cc: farosas, richard.henderson, luis.pires, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

This commit attempts to fix the first bug mentioned by Richard Henderson in
https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06247.html

To sumarize the bug here, when radix-style mmus are translating an
address, they might need to call a second level of translation, with
hypervisor privileges. However, the way it was being done up until
this point meant that the second level translation had the same
privileges as the first level. 

This patch attempts to correct that by making radix64_*_xlate functions
receive the mmu_idx, and passing one with the correct permission for the
second level translation.

The mmuidx macros added by this patch are only correct for non-bookE
mmus, because BookE style set the IS and DS bits inverted and there
might be other subtle differences. However, there doesn't seem to be
BookE cpus that have radix-style mmus, so we left a comment there to
document the issue, in case a machine does have that and was missed.

As part of this cleanup, we now need to send the correct mmmu_idx
when calling get_phys_page_debug, otherwise we might not be able to see the
memory that the CPU could

Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/ppc/internal.h    | 13 +++++++++++++
 target/ppc/mmu-radix64.c | 37 +++++++++++++++++++++----------------
 target/ppc/mmu-radix64.h |  2 +-
 target/ppc/mmu_helper.c  |  8 +++++---
 4 files changed, 40 insertions(+), 20 deletions(-)

diff --git a/target/ppc/internal.h b/target/ppc/internal.h
index f1fd3c8d04..11a0e22cc9 100644
--- a/target/ppc/internal.h
+++ b/target/ppc/internal.h
@@ -245,4 +245,17 @@ static inline int prot_for_access_type(MMUAccessType access_type)
     g_assert_not_reached();
 }
 
+/*
+ * These correspond to the mmu_idx values computed in
+ * hreg_compute_hflags_value. See the tables therein
+ */
+static inline bool mmuidx_pr(int idx) { return !(idx & 1); }
+/*
+ * This macro is only correct for non Book-E MMUs. We can add an if clause
+ * to check for mmu model, but since those don't have the bug, we decided to
+ * keep the code clean.
+ */
+static inline bool mmuidx_real(int idx) { return idx & 2; }
+static inline bool mmuidx_hv(int idx) { return idx & 4; }
+
 #endif /* PPC_INTERNAL_H */
diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
index cbd404bfa4..5b0e62e676 100644
--- a/target/ppc/mmu-radix64.c
+++ b/target/ppc/mmu-radix64.c
@@ -155,7 +155,7 @@ static void ppc_radix64_raise_hsi(PowerPCCPU *cpu, MMUAccessType access_type,
 
 static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType access_type,
                                    uint64_t pte, int *fault_cause, int *prot,
-                                   bool partition_scoped)
+                                   int mmu_idx, bool partition_scoped)
 {
     CPUPPCState *env = &cpu->env;
     int need_prot;
@@ -173,7 +173,8 @@ static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType access_type,
     /* Determine permissions allowed by Encoded Access Authority */
     if (!partition_scoped && (pte & R_PTE_EAA_PRIV) && msr_pr) {
         *prot = 0;
-    } else if (msr_pr || (pte & R_PTE_EAA_PRIV) || partition_scoped) {
+    } else if (mmuidx_pr(mmu_idx) || (pte & R_PTE_EAA_PRIV) ||
+               partition_scoped) {
         *prot = ppc_radix64_get_prot_eaa(pte);
     } else { /* !msr_pr && !(pte & R_PTE_EAA_PRIV) && !partition_scoped */
         *prot = ppc_radix64_get_prot_eaa(pte);
@@ -299,7 +300,7 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu,
                                               ppc_v3_pate_t pate,
                                               hwaddr *h_raddr, int *h_prot,
                                               int *h_page_size, bool pde_addr,
-                                              bool guest_visible)
+                                              int mmu_idx, bool guest_visible)
 {
     int fault_cause = 0;
     hwaddr pte_addr;
@@ -310,7 +311,8 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu,
     if (ppc_radix64_walk_tree(CPU(cpu)->as, g_raddr, pate.dw0 & PRTBE_R_RPDB,
                               pate.dw0 & PRTBE_R_RPDS, h_raddr, h_page_size,
                               &pte, &fault_cause, &pte_addr) ||
-        ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause, h_prot, true)) {
+        ppc_radix64_check_prot(cpu, access_type, pte,
+                               &fault_cause, h_prot, mmu_idx, true)) {
         if (pde_addr) { /* address being translated was that of a guest pde */
             fault_cause |= DSISR_PRTABLE_FAULT;
         }
@@ -332,7 +334,7 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
                                             vaddr eaddr, uint64_t pid,
                                             ppc_v3_pate_t pate, hwaddr *g_raddr,
                                             int *g_prot, int *g_page_size,
-                                            bool guest_visible)
+                                            int mmu_idx, bool guest_visible)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -367,7 +369,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
         ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, prtbe_addr,
                                                  pate, &h_raddr, &h_prot,
                                                  &h_page_size, true,
-                                                 guest_visible);
+            /* mmu_idx is 5 because we're translating from hypervisor scope */
+                                                 5, guest_visible);
         if (ret) {
             return ret;
         }
@@ -407,7 +410,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
             ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, pte_addr,
                                                      pate, &h_raddr, &h_prot,
                                                      &h_page_size, true,
-                                                     guest_visible);
+            /* mmu_idx is 5 because we're translating from hypervisor scope */
+                                                     5, guest_visible);
             if (ret) {
                 return ret;
             }
@@ -431,7 +435,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
         *g_raddr = (rpn & ~mask) | (eaddr & mask);
     }
 
-    if (ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause, g_prot, false)) {
+    if (ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause,
+                               g_prot, mmu_idx, false)) {
         /* Access denied due to protection */
         if (guest_visible) {
             ppc_radix64_raise_si(cpu, access_type, eaddr, fault_cause);
@@ -464,7 +469,7 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
  *              +-------------+----------------+---------------+
  */
 bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
-                       hwaddr *raddr, int *psizep, int *protp,
+                       hwaddr *raddr, int *psizep, int *protp, int mmu_idx,
                        bool guest_visible)
 {
     CPUPPCState *env = &cpu->env;
@@ -474,17 +479,17 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
     hwaddr g_raddr;
     bool relocation;
 
-    assert(!(msr_hv && cpu->vhyp));
+    assert(!(mmuidx_hv(mmu_idx) && cpu->vhyp));
 
-    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
+    relocation = !mmuidx_real(mmu_idx);
 
     /* HV or virtual hypervisor Real Mode Access */
-    if (!relocation && (msr_hv || cpu->vhyp)) {
+    if (!relocation && (mmuidx_hv(mmu_idx) || cpu->vhyp)) {
         /* In real mode top 4 effective addr bits (mostly) ignored */
         *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
 
         /* In HV mode, add HRMOR if top EA bit is clear */
-        if (msr_hv || !env->has_hv_mode) {
+        if (mmuidx_hv(mmu_idx) || !env->has_hv_mode) {
             if (!(eaddr >> 63)) {
                 *raddr |= env->spr[SPR_HRMOR];
            }
@@ -546,7 +551,7 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
     if (relocation) {
         int ret = ppc_radix64_process_scoped_xlate(cpu, access_type, eaddr, pid,
                                                    pate, &g_raddr, &prot,
-                                                   &psize, guest_visible);
+                                                   &psize, mmu_idx, guest_visible);
         if (ret) {
             return false;
         }
@@ -564,13 +569,13 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
          * quadrants 1 or 2. Translates a guest real address to a host
          * real address.
          */
-        if (lpid || !msr_hv) {
+        if (lpid || !mmuidx_hv(mmu_idx)) {
             int ret;
 
             ret = ppc_radix64_partition_scoped_xlate(cpu, access_type, eaddr,
                                                      g_raddr, pate, raddr,
                                                      &prot, &psize, false,
-                                                     guest_visible);
+                                                     mmu_idx, guest_visible);
             if (ret) {
                 return false;
             }
diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
index 6b13b89b64..b70357cf34 100644
--- a/target/ppc/mmu-radix64.h
+++ b/target/ppc/mmu-radix64.h
@@ -45,7 +45,7 @@
 #ifdef TARGET_PPC64
 
 bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
-                       hwaddr *raddr, int *psizep, int *protp,
+                       hwaddr *raddr, int *psizep, int *protp, int mmu_idx,
                        bool guest_visible);
 
 static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
index ba1952c77d..9dcdf88597 100644
--- a/target/ppc/mmu_helper.c
+++ b/target/ppc/mmu_helper.c
@@ -2908,7 +2908,7 @@ static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
     case POWERPC_MMU_3_00:
         if (ppc64_v3_radix(cpu)) {
             return ppc_radix64_xlate(cpu, eaddr, access_type,
-                                     raddrp, psizep, protp, guest_visible);
+                                     raddrp, psizep, protp, mmu_idx, guest_visible);
         }
         /* fall through */
     case POWERPC_MMU_64B:
@@ -2941,8 +2941,10 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
      * try an MMU_DATA_LOAD, we may not be able to read instructions
      * mapped by code TLBs, so we also try a MMU_INST_FETCH.
      */
-    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
-        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
+    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p,
+                  cpu_mmu_index(&cpu->env, false), false) ||
+        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p,
+                  cpu_mmu_index(&cpu->env, true), false)) {
         return raddr & TARGET_PAGE_MASK;
     }
     return -1;
-- 
2.17.1



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

* Re: [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault
  2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
@ 2021-06-22 10:49   ` Greg Kurz
  2021-06-24  1:40   ` David Gibson
  1 sibling, 0 replies; 22+ messages in thread
From: Greg Kurz @ 2021-06-22 10:49 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, luis.pires, qemu-devel, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

On Mon, 21 Jun 2021 09:51:06 -0300
"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> wrote:

> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Instead, use a switch on env->mmu_model.  This avoids some
> replicated information in cpu setup.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Very nice !

Reviewed-by: Greg Kurz <groug@kaod.org>

>  target/ppc/cpu-qom.h    |  1 -
>  target/ppc/cpu_init.c   | 45 -----------------------------------------
>  target/ppc/mmu_helper.c | 24 ++++++++++++++++++----
>  3 files changed, 20 insertions(+), 50 deletions(-)
> 
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 06b6571bc9..3b14d2f134 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -198,7 +198,6 @@ struct PowerPCCPUClass {
>      int n_host_threads;
>      void (*init_proc)(CPUPPCState *env);
>      int  (*check_pow)(CPUPPCState *env);
> -    int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int mmu_idx);
>      bool (*interrupts_big_endian)(PowerPCCPU *cpu);
>  };
>  
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index d0411e7302..3a8d8d3f07 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -4578,9 +4578,6 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);
>      pcc->mmu_model = POWERPC_MMU_601;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_601;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_601;
> @@ -4623,9 +4620,6 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);
>      pcc->mmu_model = POWERPC_MMU_601;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_601;
>      pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> @@ -4889,9 +4883,6 @@ POWERPC_FAMILY(604)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_604;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_604;
> @@ -4973,9 +4964,6 @@ POWERPC_FAMILY(604E)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_604;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_604;
> @@ -5044,9 +5032,6 @@ POWERPC_FAMILY(740)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5124,9 +5109,6 @@ POWERPC_FAMILY(750)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5327,9 +5309,6 @@ POWERPC_FAMILY(750cl)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5410,9 +5389,6 @@ POWERPC_FAMILY(750cx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5498,9 +5474,6 @@ POWERPC_FAMILY(750fx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5586,9 +5559,6 @@ POWERPC_FAMILY(750gx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5828,9 +5798,6 @@ POWERPC_FAMILY(7400)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -5914,9 +5881,6 @@ POWERPC_FAMILY(7410)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -6743,9 +6707,6 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -7505,7 +7466,6 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI);
>      pcc->mmu_model = POWERPC_MMU_64B;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_basic;
>  #endif
>      pcc->excp_model = POWERPC_EXCP_970;
> @@ -7583,7 +7543,6 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
>          LPCR_RMI | LPCR_HDICE;
>      pcc->mmu_model = POWERPC_MMU_2_03;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_basic;
>      pcc->lrg_decr_bits = 32;
>  #endif
> @@ -7727,7 +7686,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2;
>      pcc->mmu_model = POWERPC_MMU_2_06;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->lrg_decr_bits = 32;
>  #endif
> @@ -7904,7 +7862,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                     LPCR_P8_PECE3 | LPCR_P8_PECE4;
>      pcc->mmu_model = POWERPC_MMU_2_07;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->lrg_decr_bits = 32;
>      pcc->n_host_threads = 8;
> @@ -8120,7 +8077,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>      pcc->mmu_model = POWERPC_MMU_3_00;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
>      /* segment page size remain the same */
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->radix_page_info = &POWER9_radix_page_info;
> @@ -8332,7 +8288,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>      pcc->mmu_model = POWERPC_MMU_3_00;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
>      /* segment page size remain the same */
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->radix_page_info = &POWER10_radix_page_info;
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 1ecb36e85a..c4b1c93e47 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2947,14 +2947,30 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
>                        bool probe, uintptr_t retaddr)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
>      CPUPPCState *env = &cpu->env;
>      int ret;
>  
> -    if (pcc->handle_mmu_fault) {
> -        ret = pcc->handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -    } else {
> +    switch (env->mmu_model) {
> +#if defined(TARGET_PPC64)
> +    case POWERPC_MMU_64B:
> +    case POWERPC_MMU_2_03:
> +    case POWERPC_MMU_2_06:
> +    case POWERPC_MMU_2_07:
> +        ret = ppc_hash64_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +    case POWERPC_MMU_3_00:
> +        ret = ppc64_v3_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +#endif
> +
> +    case POWERPC_MMU_32B:
> +    case POWERPC_MMU_601:
> +        ret = ppc_hash32_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +
> +    default:
>          ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
> +        break;
>      }
>      if (unlikely(ret != 0)) {
>          if (probe) {



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

* Re: [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault
  2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
@ 2021-06-22 12:05   ` Greg Kurz
  2021-06-24  3:19   ` David Gibson
  1 sibling, 0 replies; 22+ messages in thread
From: Greg Kurz @ 2021-06-22 12:05 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, luis.pires, qemu-devel, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, david

On Mon, 21 Jun 2021 09:51:07 -0300
"Bruno Larsen (billionai)" <bruno.larsen@eldorado.org.br> wrote:

> From: Richard Henderson <richard.henderson@linaro.org>
> 
> These changes were waiting until we didn't need to match
> the function type of PowerPCCPUClass.handle_mmu_fault.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---

Reviewed-by: Greg Kurz <groug@kaod.org>

>  target/ppc/mmu-hash32.c  | 7 ++-----
>  target/ppc/mmu-hash32.h  | 4 ++--
>  target/ppc/mmu-hash64.c  | 6 +-----
>  target/ppc/mmu-hash64.h  | 4 ++--
>  target/ppc/mmu-radix64.c | 7 ++-----
>  target/ppc/mmu-radix64.h | 4 ++--
>  6 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 9f0a497657..8f19b43e47 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -415,8 +415,8 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
>      return (rpn & ~mask) | (eaddr & mask);
>  }
>  
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                int mmu_idx)
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -425,11 +425,8 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>      ppc_hash_pte32_t pte;
>      int prot;
>      int need_prot;
> -    MMUAccessType access_type;
>      hwaddr raddr;
>  
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
>      need_prot = prot_for_access_type(access_type);
>  
>      /* 1. Handle real mode accesses */
> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> index 898021f0d8..30e35718a7 100644
> --- a/target/ppc/mmu-hash32.h
> +++ b/target/ppc/mmu-hash32.h
> @@ -5,8 +5,8 @@
>  
>  hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
>  hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> -                                int mmu_idx);
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> +                                MMUAccessType access_type, int mmu_idx);
>  
>  /*
>   * Segment register definitions
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 708dffc31b..2febd369b1 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -874,7 +874,7 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
>  }
>  
>  int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                int rwx, int mmu_idx)
> +                                MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -884,13 +884,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>      hwaddr ptex;
>      ppc_hash_pte64_t pte;
>      int exec_prot, pp_prot, amr_prot, prot;
> -    MMUAccessType access_type;
>      int need_prot;
>      hwaddr raddr;
>  
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
> -
>      /*
>       * Note on LPCR usage: 970 uses HID4, but our special variant of
>       * store_spr copies relevant fields into env->spr[SPR_LPCR].
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 4b8b8e7950..3e8a8eec1f 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -8,8 +8,8 @@ void dump_slb(PowerPCCPU *cpu);
>  int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>                    target_ulong esid, target_ulong vsid);
>  hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> -                                int mmu_idx);
> +int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> +                                MMUAccessType access_type, int mmu_idx);
>  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
>                                 target_ulong pte_index,
>                                 target_ulong pte0, target_ulong pte1);
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index b6d191c1d8..1c707d387d 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -555,19 +555,16 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>      return 0;
>  }
>  
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                 int mmu_idx)
> +int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                 MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
>      int page_size, prot;
>      bool relocation;
> -    MMUAccessType access_type;
>      hwaddr raddr;
>  
>      assert(!(msr_hv && cpu->vhyp));
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
>  
>      relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
>      /* HV or virtual hypervisor Real Mode Access */
> diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
> index f28c5794d0..94bd72cb38 100644
> --- a/target/ppc/mmu-radix64.h
> +++ b/target/ppc/mmu-radix64.h
> @@ -44,8 +44,8 @@
>  
>  #ifdef TARGET_PPC64
>  
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                 int mmu_idx);
> +int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                 MMUAccessType access_type, int mmu_idx);
>  hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
>  
>  static inline int ppc_radix64_get_prot_eaa(uint64_t pte)



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

* Re: [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault
  2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
  2021-06-22 10:49   ` Greg Kurz
@ 2021-06-24  1:40   ` David Gibson
  1 sibling, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  1:40 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 11203 bytes --]

On Mon, Jun 21, 2021 at 09:51:06AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Instead, use a switch on env->mmu_model.  This avoids some
> replicated information in cpu setup.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

As I mentioned on the earlier posting, I don't love the overall
direction of this change (I want to move more towards hooks and things
in the cpu class than magical model constants and switches all over
the place).

But.. the cleanups this allows in the short term are worth the small
step backwards.  So, applied to ppc-for-6.1.

> ---
>  target/ppc/cpu-qom.h    |  1 -
>  target/ppc/cpu_init.c   | 45 -----------------------------------------
>  target/ppc/mmu_helper.c | 24 ++++++++++++++++++----
>  3 files changed, 20 insertions(+), 50 deletions(-)
> 
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 06b6571bc9..3b14d2f134 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -198,7 +198,6 @@ struct PowerPCCPUClass {
>      int n_host_threads;
>      void (*init_proc)(CPUPPCState *env);
>      int  (*check_pow)(CPUPPCState *env);
> -    int (*handle_mmu_fault)(PowerPCCPU *cpu, vaddr eaddr, int rwx, int mmu_idx);
>      bool (*interrupts_big_endian)(PowerPCCPU *cpu);
>  };
>  
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index d0411e7302..3a8d8d3f07 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -4578,9 +4578,6 @@ POWERPC_FAMILY(601)(ObjectClass *oc, void *data)
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);
>      pcc->mmu_model = POWERPC_MMU_601;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_601;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_601;
> @@ -4623,9 +4620,6 @@ POWERPC_FAMILY(601v)(ObjectClass *oc, void *data)
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);
>      pcc->mmu_model = POWERPC_MMU_601;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_601;
>      pcc->flags = POWERPC_FLAG_SE | POWERPC_FLAG_RTC_CLK | POWERPC_FLAG_HID0_LE;
> @@ -4889,9 +4883,6 @@ POWERPC_FAMILY(604)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_604;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_604;
> @@ -4973,9 +4964,6 @@ POWERPC_FAMILY(604E)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_604;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_604;
> @@ -5044,9 +5032,6 @@ POWERPC_FAMILY(740)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5124,9 +5109,6 @@ POWERPC_FAMILY(750)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5327,9 +5309,6 @@ POWERPC_FAMILY(750cl)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5410,9 +5389,6 @@ POWERPC_FAMILY(750cx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5498,9 +5474,6 @@ POWERPC_FAMILY(750fx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5586,9 +5559,6 @@ POWERPC_FAMILY(750gx)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_7x0;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_750;
> @@ -5828,9 +5798,6 @@ POWERPC_FAMILY(7400)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -5914,9 +5881,6 @@ POWERPC_FAMILY(7410)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -6743,9 +6707,6 @@ POWERPC_FAMILY(e600)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI) |
>                      (1ull << MSR_LE);
>      pcc->mmu_model = POWERPC_MMU_32B;
> -#if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash32_handle_mmu_fault;
> -#endif
>      pcc->excp_model = POWERPC_EXCP_74xx;
>      pcc->bus_model = PPC_FLAGS_INPUT_6xx;
>      pcc->bfd_mach = bfd_mach_ppc_7400;
> @@ -7505,7 +7466,6 @@ POWERPC_FAMILY(970)(ObjectClass *oc, void *data)
>                      (1ull << MSR_RI);
>      pcc->mmu_model = POWERPC_MMU_64B;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_basic;
>  #endif
>      pcc->excp_model = POWERPC_EXCP_970;
> @@ -7583,7 +7543,6 @@ POWERPC_FAMILY(POWER5P)(ObjectClass *oc, void *data)
>          LPCR_RMI | LPCR_HDICE;
>      pcc->mmu_model = POWERPC_MMU_2_03;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_basic;
>      pcc->lrg_decr_bits = 32;
>  #endif
> @@ -7727,7 +7686,6 @@ POWERPC_FAMILY(POWER7)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_P7_PECE0 | LPCR_P7_PECE1 | LPCR_P7_PECE2;
>      pcc->mmu_model = POWERPC_MMU_2_06;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->lrg_decr_bits = 32;
>  #endif
> @@ -7904,7 +7862,6 @@ POWERPC_FAMILY(POWER8)(ObjectClass *oc, void *data)
>                     LPCR_P8_PECE3 | LPCR_P8_PECE4;
>      pcc->mmu_model = POWERPC_MMU_2_07;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc_hash64_handle_mmu_fault;
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->lrg_decr_bits = 32;
>      pcc->n_host_threads = 8;
> @@ -8120,7 +8077,6 @@ POWERPC_FAMILY(POWER9)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>      pcc->mmu_model = POWERPC_MMU_3_00;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
>      /* segment page size remain the same */
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->radix_page_info = &POWER9_radix_page_info;
> @@ -8332,7 +8288,6 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>      pcc->lpcr_pm = LPCR_PDEE | LPCR_HDEE | LPCR_EEE | LPCR_DEE | LPCR_OEE;
>      pcc->mmu_model = POWERPC_MMU_3_00;
>  #if defined(CONFIG_SOFTMMU)
> -    pcc->handle_mmu_fault = ppc64_v3_handle_mmu_fault;
>      /* segment page size remain the same */
>      pcc->hash64_opts = &ppc_hash64_opts_POWER7;
>      pcc->radix_page_info = &POWER10_radix_page_info;
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 1ecb36e85a..c4b1c93e47 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2947,14 +2947,30 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
>                        bool probe, uintptr_t retaddr)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> -    PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cs);
>      CPUPPCState *env = &cpu->env;
>      int ret;
>  
> -    if (pcc->handle_mmu_fault) {
> -        ret = pcc->handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -    } else {
> +    switch (env->mmu_model) {
> +#if defined(TARGET_PPC64)
> +    case POWERPC_MMU_64B:
> +    case POWERPC_MMU_2_03:
> +    case POWERPC_MMU_2_06:
> +    case POWERPC_MMU_2_07:
> +        ret = ppc_hash64_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +    case POWERPC_MMU_3_00:
> +        ret = ppc64_v3_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +#endif
> +
> +    case POWERPC_MMU_32B:
> +    case POWERPC_MMU_601:
> +        ret = ppc_hash32_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> +        break;
> +
> +    default:
>          ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
> +        break;
>      }
>      if (unlikely(ret != 0)) {
>          if (probe) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault
  2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
  2021-06-22 12:05   ` Greg Kurz
@ 2021-06-24  3:19   ` David Gibson
  1 sibling, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  3:19 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 6181 bytes --]

On Mon, Jun 21, 2021 at 09:51:07AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> These changes were waiting until we didn't need to match
> the function type of PowerPCCPUClass.handle_mmu_fault.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-hash32.c  | 7 ++-----
>  target/ppc/mmu-hash32.h  | 4 ++--
>  target/ppc/mmu-hash64.c  | 6 +-----
>  target/ppc/mmu-hash64.h  | 4 ++--
>  target/ppc/mmu-radix64.c | 7 ++-----
>  target/ppc/mmu-radix64.h | 4 ++--
>  6 files changed, 11 insertions(+), 21 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index 9f0a497657..8f19b43e47 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -415,8 +415,8 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
>      return (rpn & ~mask) | (eaddr & mask);
>  }
>  
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                int mmu_idx)
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -425,11 +425,8 @@ int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
>      ppc_hash_pte32_t pte;
>      int prot;
>      int need_prot;
> -    MMUAccessType access_type;
>      hwaddr raddr;
>  
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
>      need_prot = prot_for_access_type(access_type);
>  
>      /* 1. Handle real mode accesses */
> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> index 898021f0d8..30e35718a7 100644
> --- a/target/ppc/mmu-hash32.h
> +++ b/target/ppc/mmu-hash32.h
> @@ -5,8 +5,8 @@
>  
>  hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
>  hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> -                                int mmu_idx);
> +int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> +                                MMUAccessType access_type, int mmu_idx);
>  
>  /*
>   * Segment register definitions
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 708dffc31b..2febd369b1 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -874,7 +874,7 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
>  }
>  
>  int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                int rwx, int mmu_idx)
> +                                MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -884,13 +884,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>      hwaddr ptex;
>      ppc_hash_pte64_t pte;
>      int exec_prot, pp_prot, amr_prot, prot;
> -    MMUAccessType access_type;
>      int need_prot;
>      hwaddr raddr;
>  
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
> -
>      /*
>       * Note on LPCR usage: 970 uses HID4, but our special variant of
>       * store_spr copies relevant fields into env->spr[SPR_LPCR].
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 4b8b8e7950..3e8a8eec1f 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -8,8 +8,8 @@ void dump_slb(PowerPCCPU *cpu);
>  int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>                    target_ulong esid, target_ulong vsid);
>  hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address, int rw,
> -                                int mmu_idx);
> +int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> +                                MMUAccessType access_type, int mmu_idx);
>  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
>                                 target_ulong pte_index,
>                                 target_ulong pte0, target_ulong pte1);
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index b6d191c1d8..1c707d387d 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -555,19 +555,16 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>      return 0;
>  }
>  
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                 int mmu_idx)
> +int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                 MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
>      int page_size, prot;
>      bool relocation;
> -    MMUAccessType access_type;
>      hwaddr raddr;
>  
>      assert(!(msr_hv && cpu->vhyp));
> -    assert((rwx == 0) || (rwx == 1) || (rwx == 2));
> -    access_type = rwx;
>  
>      relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
>      /* HV or virtual hypervisor Real Mode Access */
> diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
> index f28c5794d0..94bd72cb38 100644
> --- a/target/ppc/mmu-radix64.h
> +++ b/target/ppc/mmu-radix64.h
> @@ -44,8 +44,8 @@
>  
>  #ifdef TARGET_PPC64
>  
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                                 int mmu_idx);
> +int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                 MMUAccessType access_type, int mmu_idx);
>  hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
>  
>  static inline int ppc_radix64_get_prot_eaa(uint64_t pte)

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate
  2021-06-21 12:51 ` [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate Bruno Larsen (billionai)
@ 2021-06-24  3:29   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  3:29 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 5516 bytes --]

On Mon, Jun 21, 2021 at 09:51:08AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> This removes some incomplete duplication between
> ppc_radix64_handle_mmu_fault and ppc_radix64_get_phys_page_debug.
> The former was correct wrt SPR_HRMOR and the latter was not.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-radix64.c | 77 ++++++++++++++++++----------------------
>  1 file changed, 34 insertions(+), 43 deletions(-)
> 
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index 1c707d387d..dd5ae69052 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -465,7 +465,6 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>   */
>  static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>                               MMUAccessType access_type,
> -                             bool relocation,
>                               hwaddr *raddr, int *psizep, int *protp,
>                               bool guest_visible)
>  {
> @@ -474,6 +473,37 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>      ppc_v3_pate_t pate;
>      int psize, prot;
>      hwaddr g_raddr;
> +    bool relocation;
> +
> +    assert(!(msr_hv && cpu->vhyp));
> +
> +    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> +
> +    /* HV or virtual hypervisor Real Mode Access */
> +    if (!relocation && (msr_hv || cpu->vhyp)) {
> +        /* In real mode top 4 effective addr bits (mostly) ignored */
> +        *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> +
> +        /* In HV mode, add HRMOR if top EA bit is clear */
> +        if (msr_hv || !env->has_hv_mode) {

Not in scope, because this is a code motion, but that test looks
bogus.  If we don't have an HV mode, we won't have an HRMOR either.

> +            if (!(eaddr >> 63)) {
> +                *raddr |= env->spr[SPR_HRMOR];
> +           }
> +        }
> +        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> +        *psizep = TARGET_PAGE_BITS;
> +        return 0;
> +    }
> +
> +    /*
> +     * Check UPRT (we avoid the check in real mode to deal with
> +     * transitional states during kexec.
> +     */
> +    if (guest_visible && !ppc64_use_proc_tbl(cpu)) {
> +        qemu_log_mask(LOG_GUEST_ERROR,
> +                      "LPCR:UPRT not set in radix mode ! LPCR="
> +                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> +    }
>  
>      /* Virtual Mode Access - get the fully qualified address */
>      if (!ppc_radix64_get_fully_qualified_addr(&cpu->env, eaddr, &lpid, &pid)) {
> @@ -559,43 +589,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>                                   MMUAccessType access_type, int mmu_idx)
>  {
>      CPUState *cs = CPU(cpu);
> -    CPUPPCState *env = &cpu->env;
>      int page_size, prot;
> -    bool relocation;
>      hwaddr raddr;
>  
> -    assert(!(msr_hv && cpu->vhyp));
> -
> -    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> -    /* HV or virtual hypervisor Real Mode Access */
> -    if (!relocation && (msr_hv || cpu->vhyp)) {
> -        /* In real mode top 4 effective addr bits (mostly) ignored */
> -        raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
> -
> -        /* In HV mode, add HRMOR if top EA bit is clear */
> -        if (msr_hv || !env->has_hv_mode) {
> -            if (!(eaddr >> 63)) {
> -                raddr |= env->spr[SPR_HRMOR];
> -           }
> -        }
> -        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
> -                     TARGET_PAGE_SIZE);
> -        return 0;
> -    }
> -
> -    /*
> -     * Check UPRT (we avoid the check in real mode to deal with
> -     * transitional states during kexec.
> -     */
> -    if (!ppc64_use_proc_tbl(cpu)) {
> -        qemu_log_mask(LOG_GUEST_ERROR,
> -                      "LPCR:UPRT not set in radix mode ! LPCR="
> -                      TARGET_FMT_lx "\n", env->spr[SPR_LPCR]);
> -    }
> -
>      /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> -    if (ppc_radix64_xlate(cpu, eaddr, access_type, relocation, &raddr,
> +    if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
>                            &page_size, &prot, true)) {
>          return 1;
>      }
> @@ -607,18 +605,11 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>  
>  hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
>  {
> -    CPUPPCState *env = &cpu->env;
>      int psize, prot;
>      hwaddr raddr;
>  
> -    /* Handle Real Mode */
> -    if ((msr_dr == 0) && (msr_hv || cpu->vhyp)) {
> -        /* In real mode top 4 effective addr bits (mostly) ignored */
> -        return eaddr & 0x0FFFFFFFFFFFFFFFULL;
> -    }
> -
> -    if (ppc_radix64_xlate(cpu, eaddr, 0, msr_dr, &raddr, &psize,
> -                          &prot, false)) {
> +    if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> +                          &psize, &prot, false)) {
>          return -1;
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate
  2021-06-21 12:51 ` [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate Bruno Larsen (billionai)
@ 2021-06-24  3:31   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  3:31 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 4710 bytes --]

On Mon, Jun 21, 2021 at 09:51:09AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Instead of returning non-zero for failure, return true for success.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-radix64.c | 30 +++++++++++++++---------------
>  1 file changed, 15 insertions(+), 15 deletions(-)
> 
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index dd5ae69052..2d5f0850c9 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -463,10 +463,10 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>   *              | = On        | Process Scoped |    Scoped     |
>   *              +-------------+----------------+---------------+
>   */
> -static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> -                             MMUAccessType access_type,
> -                             hwaddr *raddr, int *psizep, int *protp,
> -                             bool guest_visible)
> +static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> +                              MMUAccessType access_type,
> +                              hwaddr *raddr, int *psizep, int *protp,
> +                              bool guest_visible)
>  {
>      CPUPPCState *env = &cpu->env;
>      uint64_t lpid, pid;
> @@ -492,7 +492,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>          }
>          *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
>          *psizep = TARGET_PAGE_BITS;
> -        return 0;
> +        return true;
>      }
>  
>      /*
> @@ -510,7 +510,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>          if (guest_visible) {
>              ppc_radix64_raise_segi(cpu, access_type, eaddr);
>          }
> -        return 1;
> +        return false;
>      }
>  
>      /* Get Process Table */
> @@ -523,13 +523,13 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>              if (guest_visible) {
>                  ppc_radix64_raise_si(cpu, access_type, eaddr, DSISR_NOPTE);
>              }
> -            return 1;
> +            return false;
>          }
>          if (!validate_pate(cpu, lpid, &pate)) {
>              if (guest_visible) {
>                  ppc_radix64_raise_si(cpu, access_type, eaddr, DSISR_R_BADCONFIG);
>              }
> -            return 1;
> +            return false;
>          }
>      }
>  
> @@ -549,7 +549,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>                                                     pate, &g_raddr, &prot,
>                                                     &psize, guest_visible);
>          if (ret) {
> -            return ret;
> +            return false;
>          }
>          *psizep = MIN(*psizep, psize);
>          *protp &= prot;
> @@ -573,7 +573,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>                                                       &prot, &psize, false,
>                                                       guest_visible);
>              if (ret) {
> -                return ret;
> +                return false;
>              }
>              *psizep = MIN(*psizep, psize);
>              *protp &= prot;
> @@ -582,7 +582,7 @@ static int ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>          }
>      }
>  
> -    return 0;
> +    return true;
>  }
>  
>  int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> @@ -593,8 +593,8 @@ int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>      hwaddr raddr;
>  
>      /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> -    if (ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
> -                          &page_size, &prot, true)) {
> +    if (!ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
> +                           &page_size, &prot, true)) {
>          return 1;
>      }
>  
> @@ -608,8 +608,8 @@ hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
>      int psize, prot;
>      hwaddr raddr;
>  
> -    if (ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> -                          &psize, &prot, false)) {
> +    if (!ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> +                           &psize, &prot, false)) {
>          return -1;
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate
  2021-06-21 12:51 ` [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate Bruno Larsen (billionai)
@ 2021-06-24  5:55   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  5:55 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 8809 bytes --]

On Mon, Jun 21, 2021 at 09:51:10AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Mirror the interface of ppc_radix64_xlate, putting all of
> the logic for hash64 translation into a single function.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-hash64.c | 125 +++++++++++++++++++---------------------
>  1 file changed, 59 insertions(+), 66 deletions(-)
> 
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index 2febd369b1..c6b167b4dc 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -873,8 +873,10 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
>      return -1;
>  }
>  
> -int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                MMUAccessType access_type, int mmu_idx)
> +static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> +                             MMUAccessType access_type,
> +                             hwaddr *raddrp, int *psizep, int *protp,
> +                             bool guest_visible)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -918,9 +920,11 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>              slb = &vrma_slbe;
>              if (build_vrma_slbe(cpu, slb) != 0) {
>                  /* Invalid VRMA setup, machine check */
> -                cs->exception_index = POWERPC_EXCP_MCHECK;
> -                env->error_code = 0;
> -                return 1;
> +                if (guest_visible) {
> +                    cs->exception_index = POWERPC_EXCP_MCHECK;
> +                    env->error_code = 0;
> +                }
> +                return false;
>              }
>  
>              goto skip_slb_search;
> @@ -929,6 +933,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>  
>              /* Emulated old-style RMO mode, bounds check against RMLS */
>              if (raddr >= limit) {
> +                if (!guest_visible) {
> +                    return false;
> +                }
>                  switch (access_type) {
>                  case MMU_INST_FETCH:
>                      ppc_hash64_set_isi(cs, SRR1_PROTFAULT);
> @@ -943,15 +950,16 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>                  default:
>                      g_assert_not_reached();
>                  }
> -                return 1;
> +                return false;
>              }
>  
>              raddr |= env->spr[SPR_RMOR];
>          }
> -        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                     PAGE_READ | PAGE_WRITE | PAGE_EXEC, mmu_idx,
> -                     TARGET_PAGE_SIZE);
> -        return 0;
> +
> +        *raddrp = raddr;
> +        *protp = PAGE_READ | PAGE_WRITE | PAGE_EXEC;
> +        *psizep = TARGET_PAGE_BITS;
> +        return true;
>      }
>  
>      /* 2. Translation is on, so look up the SLB */
> @@ -964,6 +972,9 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>              exit(1);
>          }
>          /* Segment still not found, generate the appropriate interrupt */
> +        if (!guest_visible) {
> +            return false;
> +        }
>          switch (access_type) {
>          case MMU_INST_FETCH:
>              cs->exception_index = POWERPC_EXCP_ISEG;
> @@ -978,20 +989,25 @@ int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
>          default:
>              g_assert_not_reached();
>          }
> -        return 1;
> +        return false;
>      }
>  
> -skip_slb_search:
> + skip_slb_search:
>  
>      /* 3. Check for segment level no-execute violation */
>      if (access_type == MMU_INST_FETCH && (slb->vsid & SLB_VSID_N)) {
> -        ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD);
> -        return 1;
> +        if (guest_visible) {
> +            ppc_hash64_set_isi(cs, SRR1_NOEXEC_GUARD);
> +        }
> +        return false;
>      }
>  
>      /* 4. Locate the PTE in the hash table */
>      ptex = ppc_hash64_htab_lookup(cpu, slb, eaddr, &pte, &apshift);
>      if (ptex == -1) {
> +        if (!guest_visible) {
> +            return false;
> +        }
>          switch (access_type) {
>          case MMU_INST_FETCH:
>              ppc_hash64_set_isi(cs, SRR1_NOPTE);
> @@ -1005,7 +1021,7 @@ skip_slb_search:
>          default:
>              g_assert_not_reached();
>          }
> -        return 1;
> +        return false;
>      }
>      qemu_log_mask(CPU_LOG_MMU,
>                    "found PTE at index %08" HWADDR_PRIx "\n", ptex);
> @@ -1021,6 +1037,9 @@ skip_slb_search:
>      if (need_prot & ~prot) {
>          /* Access right violation */
>          qemu_log_mask(CPU_LOG_MMU, "PTE access rejected\n");
> +        if (!guest_visible) {
> +            return false;
> +        }
>          if (access_type == MMU_INST_FETCH) {
>              int srr1 = 0;
>              if (PAGE_EXEC & ~exec_prot) {
> @@ -1045,7 +1064,7 @@ skip_slb_search:
>              }
>              ppc_hash64_set_dsi(cs, eaddr, dsisr);
>          }
> -        return 1;
> +        return false;
>      }
>  
>      qemu_log_mask(CPU_LOG_MMU, "PTE access granted !\n");
> @@ -1069,66 +1088,40 @@ skip_slb_search:
>  
>      /* 7. Determine the real address from the PTE */
>  
> -    raddr = deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, eaddr);
> -
> -    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                 prot, mmu_idx, 1ULL << apshift);
> -
> -    return 0;
> +    *raddrp = deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, eaddr);
> +    *protp = prot;
> +    *psizep = apshift;
> +    return true;
>  }
>  
> -hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr)
> +int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                MMUAccessType access_type, int mmu_idx)
>  {
> -    CPUPPCState *env = &cpu->env;
> -    ppc_slb_t vrma_slbe;
> -    ppc_slb_t *slb;
> -    hwaddr ptex, raddr;
> -    ppc_hash_pte64_t pte;
> -    unsigned apshift;
> +    CPUState *cs = CPU(cpu);
> +    int page_size, prot;
> +    hwaddr raddr;
>  
> -    /* Handle real mode */
> -    if (msr_dr == 0) {
> -        /* In real mode the top 4 effective address bits are ignored */
> -        raddr = addr & 0x0FFFFFFFFFFFFFFFULL;
> +    if (!ppc_hash64_xlate(cpu, eaddr, access_type, &raddr,
> +                          &page_size, &prot, true)) {
> +        return 1;
> +    }
>  
> -        if (cpu->vhyp) {
> -            /*
> -             * In virtual hypervisor mode, there's nothing to do:
> -             *   EA == GPA == qemu guest address
> -             */
> -            return raddr;
> -        } else if ((msr_hv || !env->has_hv_mode) && !(addr >> 63)) {
> -            /* In HV mode, add HRMOR if top EA bit is clear */
> -            return raddr | env->spr[SPR_HRMOR];
> -        } else if (ppc_hash64_use_vrma(env)) {
> -            /* Emulated VRMA mode */
> -            slb = &vrma_slbe;
> -            if (build_vrma_slbe(cpu, slb) != 0) {
> -                return -1;
> -            }
> -        } else {
> -            target_ulong limit = rmls_limit(cpu);
> +    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> +                 prot, mmu_idx, 1UL << page_size);
> +    return 0;
> +}
>  
> -            /* Emulated old-style RMO mode, bounds check against RMLS */
> -            if (raddr >= limit) {
> -                return -1;
> -            }
> -            return raddr | env->spr[SPR_RMOR];
> -        }
> -    } else {
> -        slb = slb_lookup(cpu, addr);
> -        if (!slb) {
> -            return -1;
> -        }
> -    }
> +hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> +{
> +    int psize, prot;
> +    hwaddr raddr;
>  
> -    ptex = ppc_hash64_htab_lookup(cpu, slb, addr, &pte, &apshift);
> -    if (ptex == -1) {
> +    if (!ppc_hash64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> +                          &psize, &prot, false)) {
>          return -1;
>      }
>  
> -    return deposit64(pte.pte1 & HPTE64_R_RPN, 0, apshift, addr)
> -        & TARGET_PAGE_MASK;
> +    return raddr & TARGET_PAGE_MASK;
>  }
>  
>  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate
  2021-06-21 12:51 ` [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate Bruno Larsen (billionai)
@ 2021-06-24  6:30   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  6:30 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 15691 bytes --]

On Mon, Jun 21, 2021 at 09:51:12AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Mirror the interface of ppc_radix64_xlate (mostly), putting all
> of the logic for older mmu translation into a single entry point.
> For booke, we need to add mmu_idx to the xlate-style interface.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1.  So, I noticed reviewing this that the
existing debug path is wrong - a bunch of the get_physical_address()
functions make guest visible state changes.  Oh well, this doesn't
make it any worse.

> ---
>  target/ppc/mmu_helper.c | 179 +++++++++++++++++++++-------------------
>  1 file changed, 96 insertions(+), 83 deletions(-)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index c4b1c93e47..2e92deb105 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -1435,48 +1435,6 @@ static int get_physical_address(CPUPPCState *env, mmu_ctx_t *ctx,
>  }
>  #endif
>  
> -hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> -{
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> -    CPUPPCState *env = &cpu->env;
> -    mmu_ctx_t ctx;
> -
> -    switch (env->mmu_model) {
> -#if defined(TARGET_PPC64)
> -    case POWERPC_MMU_64B:
> -    case POWERPC_MMU_2_03:
> -    case POWERPC_MMU_2_06:
> -    case POWERPC_MMU_2_07:
> -        return ppc_hash64_get_phys_page_debug(cpu, addr);
> -    case POWERPC_MMU_3_00:
> -        return ppc64_v3_get_phys_page_debug(cpu, addr);
> -#endif
> -
> -    case POWERPC_MMU_32B:
> -    case POWERPC_MMU_601:
> -        return ppc_hash32_get_phys_page_debug(cpu, addr);
> -
> -    default:
> -        ;
> -    }
> -
> -    if (unlikely(get_physical_address(env, &ctx, addr, MMU_DATA_LOAD,
> -                                      ACCESS_INT) != 0)) {
> -
> -        /*
> -         * Some MMUs have separate TLBs for code and data. If we only
> -         * try an ACCESS_INT, we may not be able to read instructions
> -         * mapped by code TLBs, so we also try a ACCESS_CODE.
> -         */
> -        if (unlikely(get_physical_address(env, &ctx, addr, MMU_INST_FETCH,
> -                                          ACCESS_CODE) != 0)) {
> -            return -1;
> -        }
> -    }
> -
> -    return ctx.raddr & TARGET_PAGE_MASK;
> -}
> -
>  static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
>                                           MMUAccessType access_type, int mmu_idx)
>  {
> @@ -1532,30 +1490,38 @@ static void booke206_update_mas_tlb_miss(CPUPPCState *env, target_ulong address,
>  }
>  
>  /* Perform address translation */
> -static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
> -                                    MMUAccessType access_type, int mmu_idx)
> +/* TODO: Split this by mmu_model. */
> +static bool ppc_jumbo_xlate(PowerPCCPU *cpu, vaddr eaddr,
> +                            MMUAccessType access_type,
> +                            hwaddr *raddrp, int *psizep, int *protp,
> +                            int mmu_idx, bool guest_visible)
>  {
> -    CPUState *cs = env_cpu(env);
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
>      mmu_ctx_t ctx;
>      int type;
> -    int ret = 0;
> +    int ret;
>  
>      if (access_type == MMU_INST_FETCH) {
>          /* code access */
>          type = ACCESS_CODE;
> -    } else {
> +    } else if (guest_visible) {
>          /* data access */
>          type = env->access_type;
> +    } else {
> +        type = ACCESS_INT;
>      }
> -    ret = get_physical_address_wtlb(env, &ctx, address, access_type,
> +
> +    ret = get_physical_address_wtlb(env, &ctx, eaddr, access_type,
>                                      type, mmu_idx);
>      if (ret == 0) {
> -        tlb_set_page(cs, address & TARGET_PAGE_MASK,
> -                     ctx.raddr & TARGET_PAGE_MASK, ctx.prot,
> -                     mmu_idx, TARGET_PAGE_SIZE);
> -        ret = 0;
> -    } else if (ret < 0) {
> +        *raddrp = ctx.raddr;
> +        *protp = ctx.prot;
> +        *psizep = TARGET_PAGE_BITS;
> +        return true;
> +    }
> +
> +    if (guest_visible) {
>          LOG_MMU_STATE(cs);
>          if (type == ACCESS_CODE) {
>              switch (ret) {
> @@ -1565,7 +1531,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  case POWERPC_MMU_SOFT_6xx:
>                      cs->exception_index = POWERPC_EXCP_IFTLB;
>                      env->error_code = 1 << 18;
> -                    env->spr[SPR_IMISS] = address;
> +                    env->spr[SPR_IMISS] = eaddr;
>                      env->spr[SPR_ICMP] = 0x80000000 | ctx.ptem;
>                      goto tlb_miss;
>                  case POWERPC_MMU_SOFT_74xx:
> @@ -1575,29 +1541,25 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  case POWERPC_MMU_SOFT_4xx_Z:
>                      cs->exception_index = POWERPC_EXCP_ITLB;
>                      env->error_code = 0;
> -                    env->spr[SPR_40x_DEAR] = address;
> +                    env->spr[SPR_40x_DEAR] = eaddr;
>                      env->spr[SPR_40x_ESR] = 0x00000000;
>                      break;
>                  case POWERPC_MMU_BOOKE206:
> -                    booke206_update_mas_tlb_miss(env, address, 2, mmu_idx);
> +                    booke206_update_mas_tlb_miss(env, eaddr, 2, mmu_idx);
>                      /* fall through */
>                  case POWERPC_MMU_BOOKE:
>                      cs->exception_index = POWERPC_EXCP_ITLB;
>                      env->error_code = 0;
> -                    env->spr[SPR_BOOKE_DEAR] = address;
> +                    env->spr[SPR_BOOKE_DEAR] = eaddr;
>                      env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, MMU_DATA_LOAD);
> -                    return -1;
> +                    break;
>                  case POWERPC_MMU_MPC8xx:
> -                    /* XXX: TODO */
>                      cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
> -                    break;
>                  case POWERPC_MMU_REAL:
>                      cpu_abort(cs, "PowerPC in real mode should never raise "
>                                "any MMU exceptions\n");
> -                    return -1;
>                  default:
>                      cpu_abort(cs, "Unknown or invalid MMU model\n");
> -                    return -1;
>                  }
>                  break;
>              case -2:
> @@ -1634,7 +1596,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                          cs->exception_index = POWERPC_EXCP_DLTLB;
>                          env->error_code = 0;
>                      }
> -                    env->spr[SPR_DMISS] = address;
> +                    env->spr[SPR_DMISS] = eaddr;
>                      env->spr[SPR_DCMP] = 0x80000000 | ctx.ptem;
>                  tlb_miss:
>                      env->error_code |= ctx.key << 19;
> @@ -1652,7 +1614,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  tlb_miss_74xx:
>                      /* Implement LRU algorithm */
>                      env->error_code = ctx.key << 19;
> -                    env->spr[SPR_TLBMISS] = (address & ~((target_ulong)0x3)) |
> +                    env->spr[SPR_TLBMISS] = (eaddr & ~((target_ulong)0x3)) |
>                          ((env->last_way + 1) & (env->nb_ways - 1));
>                      env->spr[SPR_PTEHI] = 0x80000000 | ctx.ptem;
>                      break;
> @@ -1660,7 +1622,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  case POWERPC_MMU_SOFT_4xx_Z:
>                      cs->exception_index = POWERPC_EXCP_DTLB;
>                      env->error_code = 0;
> -                    env->spr[SPR_40x_DEAR] = address;
> +                    env->spr[SPR_40x_DEAR] = eaddr;
>                      if (access_type == MMU_DATA_STORE) {
>                          env->spr[SPR_40x_ESR] = 0x00800000;
>                      } else {
> @@ -1670,23 +1632,20 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  case POWERPC_MMU_MPC8xx:
>                      /* XXX: TODO */
>                      cpu_abort(cs, "MPC8xx MMU model is not implemented\n");
> -                    break;
>                  case POWERPC_MMU_BOOKE206:
> -                    booke206_update_mas_tlb_miss(env, address, access_type, mmu_idx);
> +                    booke206_update_mas_tlb_miss(env, eaddr, access_type, mmu_idx);
>                      /* fall through */
>                  case POWERPC_MMU_BOOKE:
>                      cs->exception_index = POWERPC_EXCP_DTLB;
>                      env->error_code = 0;
> -                    env->spr[SPR_BOOKE_DEAR] = address;
> +                    env->spr[SPR_BOOKE_DEAR] = eaddr;
>                      env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
> -                    return -1;
> +                    break;
>                  case POWERPC_MMU_REAL:
>                      cpu_abort(cs, "PowerPC in real mode should never raise "
>                                "any MMU exceptions\n");
> -                    return -1;
>                  default:
>                      cpu_abort(cs, "Unknown or invalid MMU model\n");
> -                    return -1;
>                  }
>                  break;
>              case -2:
> @@ -1695,16 +1654,16 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                  env->error_code = 0;
>                  if (env->mmu_model == POWERPC_MMU_SOFT_4xx
>                      || env->mmu_model == POWERPC_MMU_SOFT_4xx_Z) {
> -                    env->spr[SPR_40x_DEAR] = address;
> +                    env->spr[SPR_40x_DEAR] = eaddr;
>                      if (access_type == MMU_DATA_STORE) {
>                          env->spr[SPR_40x_ESR] |= 0x00800000;
>                      }
>                  } else if ((env->mmu_model == POWERPC_MMU_BOOKE) ||
>                             (env->mmu_model == POWERPC_MMU_BOOKE206)) {
> -                    env->spr[SPR_BOOKE_DEAR] = address;
> +                    env->spr[SPR_BOOKE_DEAR] = eaddr;
>                      env->spr[SPR_BOOKE_ESR] = mmubooke206_esr(mmu_idx, access_type);
>                  } else {
> -                    env->spr[SPR_DAR] = address;
> +                    env->spr[SPR_DAR] = eaddr;
>                      if (access_type == MMU_DATA_STORE) {
>                          env->spr[SPR_DSISR] = 0x0A000000;
>                      } else {
> @@ -1719,13 +1678,13 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                      /* Floating point load/store */
>                      cs->exception_index = POWERPC_EXCP_ALIGN;
>                      env->error_code = POWERPC_EXCP_ALIGN_FP;
> -                    env->spr[SPR_DAR] = address;
> +                    env->spr[SPR_DAR] = eaddr;
>                      break;
>                  case ACCESS_RES:
>                      /* lwarx, ldarx or stwcx. */
>                      cs->exception_index = POWERPC_EXCP_DSI;
>                      env->error_code = 0;
> -                    env->spr[SPR_DAR] = address;
> +                    env->spr[SPR_DAR] = eaddr;
>                      if (access_type == MMU_DATA_STORE) {
>                          env->spr[SPR_DSISR] = 0x06000000;
>                      } else {
> @@ -1736,7 +1695,7 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                      /* eciwx or ecowx */
>                      cs->exception_index = POWERPC_EXCP_DSI;
>                      env->error_code = 0;
> -                    env->spr[SPR_DAR] = address;
> +                    env->spr[SPR_DAR] = eaddr;
>                      if (access_type == MMU_DATA_STORE) {
>                          env->spr[SPR_DSISR] = 0x06100000;
>                      } else {
> @@ -1748,16 +1707,14 @@ static int cpu_ppc_handle_mmu_fault(CPUPPCState *env, target_ulong address,
>                      cs->exception_index = POWERPC_EXCP_PROGRAM;
>                      env->error_code =
>                          POWERPC_EXCP_INVAL | POWERPC_EXCP_INVAL_INVAL;
> -                    env->spr[SPR_DAR] = address;
> +                    env->spr[SPR_DAR] = eaddr;
>                      break;
>                  }
>                  break;
>              }
>          }
> -        ret = 1;
>      }
> -
> -    return ret;
> +    return false;
>  }
>  
>  #ifdef CONFIG_TCG
> @@ -2942,6 +2899,62 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
>  
>  /*****************************************************************************/
>  
> +static int cpu_ppc_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> +                                    MMUAccessType access_type, int mmu_idx)
> +{
> +    CPUState *cs = CPU(cpu);
> +    int page_size, prot;
> +    hwaddr raddr;
> +
> +    if (!ppc_jumbo_xlate(cpu, eaddr, access_type, &raddr,
> +                         &page_size, &prot, mmu_idx, true)) {
> +        return 1;
> +    }
> +
> +    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> +                 prot, mmu_idx, 1UL << page_size);
> +    return 0;
> +}
> +
> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    CPUPPCState *env = &cpu->env;
> +    hwaddr raddr;
> +    int s, p;
> +
> +    switch (env->mmu_model) {
> +#if defined(TARGET_PPC64)
> +    case POWERPC_MMU_64B:
> +    case POWERPC_MMU_2_03:
> +    case POWERPC_MMU_2_06:
> +    case POWERPC_MMU_2_07:
> +        return ppc_hash64_get_phys_page_debug(cpu, addr);
> +    case POWERPC_MMU_3_00:
> +        return ppc64_v3_get_phys_page_debug(cpu, addr);
> +#endif
> +
> +    case POWERPC_MMU_32B:
> +    case POWERPC_MMU_601:
> +        return ppc_hash32_get_phys_page_debug(cpu, addr);
> +
> +    default:
> +        ;
> +    }
> +
> +    /*
> +     * Some MMUs have separate TLBs for code and data. If we only
> +     * try an MMU_DATA_LOAD, we may not be able to read instructions
> +     * mapped by code TLBs, so we also try a MMU_INST_FETCH.
> +     */
> +    if (ppc_jumbo_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
> +        ppc_jumbo_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
> +        return raddr & TARGET_PAGE_MASK;
> +    }
> +    return -1;
> +}
> +
> +
>  bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
>                        MMUAccessType access_type, int mmu_idx,
>                        bool probe, uintptr_t retaddr)
> @@ -2969,7 +2982,7 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
>          break;
>  
>      default:
> -        ret = cpu_ppc_handle_mmu_fault(env, addr, access_type, mmu_idx);
> +        ret = cpu_ppc_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
>          break;
>      }
>      if (unlikely(ret != 0)) {

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 08/10] target/ppc: Introduce ppc_xlate
  2021-06-21 12:51 ` [PATCH v2 08/10] target/ppc: Introduce ppc_xlate Bruno Larsen (billionai)
@ 2021-06-24  6:34   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  6:34 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 16604 bytes --]

On Mon, Jun 21, 2021 at 09:51:13AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> Create one common dispatch for all of the ppc_*_xlate functions.
> Use ppc64_v3_radix to directly dispatch between ppc_radix64_xlate
> and ppc_hash64_xlate.
> 
> Remove the separate *_handle_mmu_fault and *_get_phys_page_debug
> functions, using common code for ppc_cpu_tlb_fill and
> ppc_cpu_get_phys_page_debug.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu-book3s-v3.c |  19 -------
>  target/ppc/mmu-book3s-v3.h |   5 --
>  target/ppc/mmu-hash32.c    |  38 ++------------
>  target/ppc/mmu-hash32.h    |   6 +--
>  target/ppc/mmu-hash64.c    |  37 ++------------
>  target/ppc/mmu-hash64.h    |   6 +--
>  target/ppc/mmu-radix64.c   |  38 ++------------
>  target/ppc/mmu-radix64.h   |   6 +--
>  target/ppc/mmu_helper.c    | 100 ++++++++++++++-----------------------
>  9 files changed, 55 insertions(+), 200 deletions(-)
> 
> diff --git a/target/ppc/mmu-book3s-v3.c b/target/ppc/mmu-book3s-v3.c
> index c78fd8dc0e..f4985bae78 100644
> --- a/target/ppc/mmu-book3s-v3.c
> +++ b/target/ppc/mmu-book3s-v3.c
> @@ -23,25 +23,6 @@
>  #include "mmu-book3s-v3.h"
>  #include "mmu-radix64.h"
>  
> -int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                              int mmu_idx)
> -{
> -    if (ppc64_v3_radix(cpu)) { /* Guest uses radix */
> -        return ppc_radix64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
> -    } else { /* Guest uses hash */
> -        return ppc_hash64_handle_mmu_fault(cpu, eaddr, rwx, mmu_idx);
> -    }
> -}
> -
> -hwaddr ppc64_v3_get_phys_page_debug(PowerPCCPU *cpu, vaddr eaddr)
> -{
> -    if (ppc64_v3_radix(cpu)) {
> -        return ppc_radix64_get_phys_page_debug(cpu, eaddr);
> -    } else {
> -        return ppc_hash64_get_phys_page_debug(cpu, eaddr);
> -    }
> -}
> -
>  bool ppc64_v3_get_pate(PowerPCCPU *cpu, target_ulong lpid, ppc_v3_pate_t *entry)
>  {
>      uint64_t patb = cpu->env.spr[SPR_PTCR] & PTCR_PATB;
> diff --git a/target/ppc/mmu-book3s-v3.h b/target/ppc/mmu-book3s-v3.h
> index 7b89be54b8..a1326df969 100644
> --- a/target/ppc/mmu-book3s-v3.h
> +++ b/target/ppc/mmu-book3s-v3.h
> @@ -67,11 +67,6 @@ static inline bool ppc64_v3_radix(PowerPCCPU *cpu)
>      return !!(cpu->env.spr[SPR_LPCR] & LPCR_HR);
>  }
>  
> -hwaddr ppc64_v3_get_phys_page_debug(PowerPCCPU *cpu, vaddr eaddr);
> -
> -int ppc64_v3_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr, int rwx,
> -                              int mmu_idx);
> -
>  static inline hwaddr ppc_hash64_hpt_base(PowerPCCPU *cpu)
>  {
>      uint64_t base;
> diff --git a/target/ppc/mmu-hash32.c b/target/ppc/mmu-hash32.c
> index ad22372c07..6a07c345e4 100644
> --- a/target/ppc/mmu-hash32.c
> +++ b/target/ppc/mmu-hash32.c
> @@ -424,10 +424,9 @@ static hwaddr ppc_hash32_pte_raddr(target_ulong sr, ppc_hash_pte32_t pte,
>      return (rpn & ~mask) | (eaddr & mask);
>  }
>  
> -static bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr,
> -                             MMUAccessType access_type,
> -                             hwaddr *raddrp, int *psizep, int *protp,
> -                             bool guest_visible)
> +bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                      hwaddr *raddrp, int *psizep, int *protp,
> +                      bool guest_visible)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -569,34 +568,3 @@ static bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr,
>      *protp = prot;
>      return true;
>  }
> -
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                MMUAccessType access_type, int mmu_idx)
> -{
> -    CPUState *cs = CPU(cpu);
> -    int page_size, prot;
> -    hwaddr raddr;
> -
> -    /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> -    if (!ppc_hash32_xlate(cpu, eaddr, access_type, &raddr,
> -                           &page_size, &prot, true)) {
> -        return 1;
> -    }
> -
> -    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                 prot, mmu_idx, 1UL << page_size);
> -    return 0;
> -}
> -
> -hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> -{
> -    int psize, prot;
> -    hwaddr raddr;
> -
> -    if (!ppc_hash32_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> -                           &psize, &prot, false)) {
> -        return -1;
> -    }
> -
> -    return raddr & TARGET_PAGE_MASK;
> -}
> diff --git a/target/ppc/mmu-hash32.h b/target/ppc/mmu-hash32.h
> index 30e35718a7..8694eccabd 100644
> --- a/target/ppc/mmu-hash32.h
> +++ b/target/ppc/mmu-hash32.h
> @@ -4,9 +4,9 @@
>  #ifndef CONFIG_USER_ONLY
>  
>  hwaddr get_pteg_offset32(PowerPCCPU *cpu, hwaddr hash);
> -hwaddr ppc_hash32_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash32_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> -                                MMUAccessType access_type, int mmu_idx);
> +bool ppc_hash32_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                      hwaddr *raddrp, int *psizep, int *protp,
> +                      bool guest_visible);
>  
>  /*
>   * Segment register definitions
> diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
> index c6b167b4dc..c1b98a97e9 100644
> --- a/target/ppc/mmu-hash64.c
> +++ b/target/ppc/mmu-hash64.c
> @@ -873,10 +873,9 @@ static int build_vrma_slbe(PowerPCCPU *cpu, ppc_slb_t *slb)
>      return -1;
>  }
>  
> -static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> -                             MMUAccessType access_type,
> -                             hwaddr *raddrp, int *psizep, int *protp,
> -                             bool guest_visible)
> +bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                      hwaddr *raddrp, int *psizep, int *protp,
> +                      bool guest_visible)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -1094,36 +1093,6 @@ static bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>      return true;
>  }
>  
> -int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                MMUAccessType access_type, int mmu_idx)
> -{
> -    CPUState *cs = CPU(cpu);
> -    int page_size, prot;
> -    hwaddr raddr;
> -
> -    if (!ppc_hash64_xlate(cpu, eaddr, access_type, &raddr,
> -                          &page_size, &prot, true)) {
> -        return 1;
> -    }
> -
> -    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                 prot, mmu_idx, 1UL << page_size);
> -    return 0;
> -}
> -
> -hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> -{
> -    int psize, prot;
> -    hwaddr raddr;
> -
> -    if (!ppc_hash64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> -                          &psize, &prot, false)) {
> -        return -1;
> -    }
> -
> -    return raddr & TARGET_PAGE_MASK;
> -}
> -
>  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu, target_ulong ptex,
>                                 target_ulong pte0, target_ulong pte1)
>  {
> diff --git a/target/ppc/mmu-hash64.h b/target/ppc/mmu-hash64.h
> index 3e8a8eec1f..9f338e1fe9 100644
> --- a/target/ppc/mmu-hash64.h
> +++ b/target/ppc/mmu-hash64.h
> @@ -7,9 +7,9 @@
>  void dump_slb(PowerPCCPU *cpu);
>  int ppc_store_slb(PowerPCCPU *cpu, target_ulong slot,
>                    target_ulong esid, target_ulong vsid);
> -hwaddr ppc_hash64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> -int ppc_hash64_handle_mmu_fault(PowerPCCPU *cpu, vaddr address,
> -                                MMUAccessType access_type, int mmu_idx);
> +bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                      hwaddr *raddrp, int *psizep, int *protp,
> +                      bool guest_visible);
>  void ppc_hash64_tlb_flush_hpte(PowerPCCPU *cpu,
>                                 target_ulong pte_index,
>                                 target_ulong pte0, target_ulong pte1);
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index 2d5f0850c9..cbd404bfa4 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -463,10 +463,9 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>   *              | = On        | Process Scoped |    Scoped     |
>   *              +-------------+----------------+---------------+
>   */
> -static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
> -                              MMUAccessType access_type,
> -                              hwaddr *raddr, int *psizep, int *protp,
> -                              bool guest_visible)
> +bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                       hwaddr *raddr, int *psizep, int *protp,
> +                       bool guest_visible)
>  {
>      CPUPPCState *env = &cpu->env;
>      uint64_t lpid, pid;
> @@ -584,34 +583,3 @@ static bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr,
>  
>      return true;
>  }
> -
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                 MMUAccessType access_type, int mmu_idx)
> -{
> -    CPUState *cs = CPU(cpu);
> -    int page_size, prot;
> -    hwaddr raddr;
> -
> -    /* Translate eaddr to raddr (where raddr is addr qemu needs for access) */
> -    if (!ppc_radix64_xlate(cpu, eaddr, access_type, &raddr,
> -                           &page_size, &prot, true)) {
> -        return 1;
> -    }
> -
> -    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                 prot, mmu_idx, 1UL << page_size);
> -    return 0;
> -}
> -
> -hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong eaddr)
> -{
> -    int psize, prot;
> -    hwaddr raddr;
> -
> -    if (!ppc_radix64_xlate(cpu, eaddr, MMU_DATA_LOAD, &raddr,
> -                           &psize, &prot, false)) {
> -        return -1;
> -    }
> -
> -    return raddr & TARGET_PAGE_MASK;
> -}
> diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
> index 94bd72cb38..6b13b89b64 100644
> --- a/target/ppc/mmu-radix64.h
> +++ b/target/ppc/mmu-radix64.h
> @@ -44,9 +44,9 @@
>  
>  #ifdef TARGET_PPC64
>  
> -int ppc_radix64_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                 MMUAccessType access_type, int mmu_idx);
> -hwaddr ppc_radix64_get_phys_page_debug(PowerPCCPU *cpu, target_ulong addr);
> +bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                       hwaddr *raddr, int *psizep, int *protp,
> +                       bool guest_visible);
>  
>  static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
>  {
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index 2e92deb105..a0e4e027d3 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2899,98 +2899,72 @@ void helper_check_tlb_flush_global(CPUPPCState *env)
>  
>  /*****************************************************************************/
>  
> -static int cpu_ppc_handle_mmu_fault(PowerPCCPU *cpu, vaddr eaddr,
> -                                    MMUAccessType access_type, int mmu_idx)
> +static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> +                      hwaddr *raddrp, int *psizep, int *protp,
> +                      int mmu_idx, bool guest_visible)
>  {
> -    CPUState *cs = CPU(cpu);
> -    int page_size, prot;
> -    hwaddr raddr;
> -
> -    if (!ppc_jumbo_xlate(cpu, eaddr, access_type, &raddr,
> -                         &page_size, &prot, mmu_idx, true)) {
> -        return 1;
> -    }
> -
> -    tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> -                 prot, mmu_idx, 1UL << page_size);
> -    return 0;
> -}
> -
> -hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> -{
> -    PowerPCCPU *cpu = POWERPC_CPU(cs);
> -    CPUPPCState *env = &cpu->env;
> -    hwaddr raddr;
> -    int s, p;
> -
> -    switch (env->mmu_model) {
> +    switch (cpu->env.mmu_model) {
>  #if defined(TARGET_PPC64)
> +    case POWERPC_MMU_3_00:
> +        if (ppc64_v3_radix(cpu)) {
> +            return ppc_radix64_xlate(cpu, eaddr, access_type,
> +                                     raddrp, psizep, protp, guest_visible);
> +        }
> +        /* fall through */
>      case POWERPC_MMU_64B:
>      case POWERPC_MMU_2_03:
>      case POWERPC_MMU_2_06:
>      case POWERPC_MMU_2_07:
> -        return ppc_hash64_get_phys_page_debug(cpu, addr);
> -    case POWERPC_MMU_3_00:
> -        return ppc64_v3_get_phys_page_debug(cpu, addr);
> +        return ppc_hash64_xlate(cpu, eaddr, access_type,
> +                                raddrp, psizep, protp, guest_visible);
>  #endif
>  
>      case POWERPC_MMU_32B:
>      case POWERPC_MMU_601:
> -        return ppc_hash32_get_phys_page_debug(cpu, addr);
> +        return ppc_hash32_xlate(cpu, eaddr, access_type,
> +                                raddrp, psizep, protp, guest_visible);
>  
>      default:
> -        ;
> +        return ppc_jumbo_xlate(cpu, eaddr, access_type, raddrp,
> +                               psizep, protp, mmu_idx, guest_visible);
>      }
> +}
> +
> +hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(cs);
> +    hwaddr raddr;
> +    int s, p;
>  
>      /*
>       * Some MMUs have separate TLBs for code and data. If we only
>       * try an MMU_DATA_LOAD, we may not be able to read instructions
>       * mapped by code TLBs, so we also try a MMU_INST_FETCH.
>       */
> -    if (ppc_jumbo_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
> -        ppc_jumbo_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
> +    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
> +        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
>          return raddr & TARGET_PAGE_MASK;
>      }
>      return -1;
>  }
>  
> -
> -bool ppc_cpu_tlb_fill(CPUState *cs, vaddr addr, int size,
> +bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
>                        MMUAccessType access_type, int mmu_idx,
>                        bool probe, uintptr_t retaddr)
>  {
>      PowerPCCPU *cpu = POWERPC_CPU(cs);
> -    CPUPPCState *env = &cpu->env;
> -    int ret;
> -
> -    switch (env->mmu_model) {
> -#if defined(TARGET_PPC64)
> -    case POWERPC_MMU_64B:
> -    case POWERPC_MMU_2_03:
> -    case POWERPC_MMU_2_06:
> -    case POWERPC_MMU_2_07:
> -        ret = ppc_hash64_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -        break;
> -    case POWERPC_MMU_3_00:
> -        ret = ppc64_v3_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -        break;
> -#endif
> -
> -    case POWERPC_MMU_32B:
> -    case POWERPC_MMU_601:
> -        ret = ppc_hash32_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -        break;
> +    hwaddr raddr;
> +    int page_size, prot;
>  
> -    default:
> -        ret = cpu_ppc_handle_mmu_fault(cpu, addr, access_type, mmu_idx);
> -        break;
> +    if (ppc_xlate(cpu, eaddr, access_type, &raddr,
> +                  &page_size, &prot, mmu_idx, !probe)) {
> +        tlb_set_page(cs, eaddr & TARGET_PAGE_MASK, raddr & TARGET_PAGE_MASK,
> +                     prot, mmu_idx, 1UL << page_size);
> +        return true;
>      }
> -    if (unlikely(ret != 0)) {
> -        if (probe) {
> -            return false;
> -        }
> -        raise_exception_err_ra(env, cs->exception_index, env->error_code,
> -                               retaddr);
> +    if (probe) {
> +        return false;
>      }
> -    return true;
> +    raise_exception_err_ra(&cpu->env, cs->exception_index,
> +                           cpu->env.error_code, retaddr);
>  }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG
  2021-06-21 12:51 ` [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG Bruno Larsen (billionai)
@ 2021-06-24  6:35   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  6:35 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, Richard Henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 1309 bytes --]

On Mon, Jun 21, 2021 at 09:51:14AM -0300, Bruno Larsen (billionai) wrote:
> From: Richard Henderson <richard.henderson@linaro.org>
> 
> This function is used by TCGCPUOps, and is thus TCG specific.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

Applied to ppc-for-6.1, thanks.

> ---
>  target/ppc/mmu_helper.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index a0e4e027d3..ba1952c77d 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2948,6 +2948,7 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>      return -1;
>  }
>  
> +#ifdef CONFIG_TCG
>  bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
>                        MMUAccessType access_type, int mmu_idx,
>                        bool probe, uintptr_t retaddr)
> @@ -2968,3 +2969,4 @@ bool ppc_cpu_tlb_fill(CPUState *cs, vaddr eaddr, int size,
>      raise_exception_err_ra(&cpu->env, cs->exception_index,
>                             cpu->env.error_code, retaddr);
>  }
> +#endif

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

* Re: [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus
  2021-06-21 12:51 ` [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
@ 2021-06-24  6:48   ` David Gibson
  0 siblings, 0 replies; 22+ messages in thread
From: David Gibson @ 2021-06-24  6:48 UTC (permalink / raw)
  To: Bruno Larsen (billionai)
  Cc: farosas, richard.henderson, qemu-devel, Greg Kurz, lucas.araujo,
	fernando.valle, qemu-ppc, clg, matheus.ferst, luis.pires

[-- Attachment #1: Type: text/plain, Size: 12964 bytes --]

On Mon, Jun 21, 2021 at 09:51:15AM -0300, Bruno Larsen (billionai) wrote:
> This commit attempts to fix the first bug mentioned by Richard Henderson in
> https://lists.nongnu.org/archive/html/qemu-devel/2021-05/msg06247.html
> 
> To sumarize the bug here, when radix-style mmus are translating an
> address, they might need to call a second level of translation, with
> hypervisor privileges. However, the way it was being done up until
> this point meant that the second level translation had the same
> privileges as the first level. 
> 
> This patch attempts to correct that by making radix64_*_xlate functions
> receive the mmu_idx, and passing one with the correct permission for the
> second level translation.
> 
> The mmuidx macros added by this patch are only correct for non-bookE
> mmus, because BookE style set the IS and DS bits inverted and there
> might be other subtle differences. However, there doesn't seem to be
> BookE cpus that have radix-style mmus, so we left a comment there to
> document the issue, in case a machine does have that and was missed.
> 
> As part of this cleanup, we now need to send the correct mmmu_idx
> when calling get_phys_page_debug, otherwise we might not be able to see the
> memory that the CPU could
> 
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/ppc/internal.h    | 13 +++++++++++++
>  target/ppc/mmu-radix64.c | 37 +++++++++++++++++++++----------------
>  target/ppc/mmu-radix64.h |  2 +-
>  target/ppc/mmu_helper.c  |  8 +++++---
>  4 files changed, 40 insertions(+), 20 deletions(-)
> 
> diff --git a/target/ppc/internal.h b/target/ppc/internal.h
> index f1fd3c8d04..11a0e22cc9 100644
> --- a/target/ppc/internal.h
> +++ b/target/ppc/internal.h
> @@ -245,4 +245,17 @@ static inline int prot_for_access_type(MMUAccessType access_type)
>      g_assert_not_reached();
>  }
>  
> +/*
> + * These correspond to the mmu_idx values computed in
> + * hreg_compute_hflags_value. See the tables therein
> + */
> +static inline bool mmuidx_pr(int idx) { return !(idx & 1); }
> +/*
> + * This macro is only correct for non Book-E MMUs. We can add an if clause
> + * to check for mmu model, but since those don't have the bug, we decided to

Referring to "the bug" in a comment isn't very helpful.

> + * keep the code clean.
> + */
> +static inline bool mmuidx_real(int idx) { return idx & 2; }
> +static inline bool mmuidx_hv(int idx) { return idx & 4; }

I'd really prefer to have these in mmu-book3s-v3.h.  Yes, I know they
cover more than bookS.  But the trouble here is that "BookE" isn't
clear: does it mean only "true" BookE, or anything that's not BookS
including 40x, 44x, and 8xx.  I don't think these are correct for all
of the 4xx variants, for one.

We can move this out later if and when we actually have users for it
outside the BookS code.

>  #endif /* PPC_INTERNAL_H */
> diff --git a/target/ppc/mmu-radix64.c b/target/ppc/mmu-radix64.c
> index cbd404bfa4..5b0e62e676 100644
> --- a/target/ppc/mmu-radix64.c
> +++ b/target/ppc/mmu-radix64.c
> @@ -155,7 +155,7 @@ static void ppc_radix64_raise_hsi(PowerPCCPU *cpu, MMUAccessType access_type,
>  
>  static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType access_type,
>                                     uint64_t pte, int *fault_cause, int *prot,
> -                                   bool partition_scoped)
> +                                   int mmu_idx, bool partition_scoped)
>  {
>      CPUPPCState *env = &cpu->env;
>      int need_prot;
> @@ -173,7 +173,8 @@ static bool ppc_radix64_check_prot(PowerPCCPU *cpu, MMUAccessType access_type,
>      /* Determine permissions allowed by Encoded Access Authority */
>      if (!partition_scoped && (pte & R_PTE_EAA_PRIV) && msr_pr) {
>          *prot = 0;
> -    } else if (msr_pr || (pte & R_PTE_EAA_PRIV) || partition_scoped) {
> +    } else if (mmuidx_pr(mmu_idx) || (pte & R_PTE_EAA_PRIV) ||
> +               partition_scoped) {

So.. it looks to me like hash64 and hash32 should also be using
mmu_idx instead of direct msr checks.  Would you care to tackle them
as well?

>          *prot = ppc_radix64_get_prot_eaa(pte);
>      } else { /* !msr_pr && !(pte & R_PTE_EAA_PRIV) && !partition_scoped */
>          *prot = ppc_radix64_get_prot_eaa(pte);
> @@ -299,7 +300,7 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu,
>                                                ppc_v3_pate_t pate,
>                                                hwaddr *h_raddr, int *h_prot,
>                                                int *h_page_size, bool pde_addr,
> -                                              bool guest_visible)
> +                                              int mmu_idx, bool guest_visible)
>  {
>      int fault_cause = 0;
>      hwaddr pte_addr;
> @@ -310,7 +311,8 @@ static int ppc_radix64_partition_scoped_xlate(PowerPCCPU *cpu,
>      if (ppc_radix64_walk_tree(CPU(cpu)->as, g_raddr, pate.dw0 & PRTBE_R_RPDB,
>                                pate.dw0 & PRTBE_R_RPDS, h_raddr, h_page_size,
>                                &pte, &fault_cause, &pte_addr) ||
> -        ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause, h_prot, true)) {
> +        ppc_radix64_check_prot(cpu, access_type, pte,
> +                               &fault_cause, h_prot, mmu_idx, true)) {
>          if (pde_addr) { /* address being translated was that of a guest pde */
>              fault_cause |= DSISR_PRTABLE_FAULT;
>          }
> @@ -332,7 +334,7 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>                                              vaddr eaddr, uint64_t pid,
>                                              ppc_v3_pate_t pate, hwaddr *g_raddr,
>                                              int *g_prot, int *g_page_size,
> -                                            bool guest_visible)
> +                                            int mmu_idx, bool guest_visible)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> @@ -367,7 +369,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>          ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, prtbe_addr,
>                                                   pate, &h_raddr, &h_prot,
>                                                   &h_page_size, true,
> -                                                 guest_visible);
> +            /* mmu_idx is 5 because we're translating from hypervisor scope */
> +                                                 5, guest_visible);
>          if (ret) {
>              return ret;
>          }
> @@ -407,7 +410,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>              ret = ppc_radix64_partition_scoped_xlate(cpu, 0, eaddr, pte_addr,
>                                                       pate, &h_raddr, &h_prot,
>                                                       &h_page_size, true,
> -                                                     guest_visible);
> +            /* mmu_idx is 5 because we're translating from hypervisor scope */
> +                                                     5, guest_visible);
>              if (ret) {
>                  return ret;
>              }
> @@ -431,7 +435,8 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>          *g_raddr = (rpn & ~mask) | (eaddr & mask);
>      }
>  
> -    if (ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause, g_prot, false)) {
> +    if (ppc_radix64_check_prot(cpu, access_type, pte, &fault_cause,
> +                               g_prot, mmu_idx, false)) {
>          /* Access denied due to protection */
>          if (guest_visible) {
>              ppc_radix64_raise_si(cpu, access_type, eaddr, fault_cause);
> @@ -464,7 +469,7 @@ static int ppc_radix64_process_scoped_xlate(PowerPCCPU *cpu,
>   *              +-------------+----------------+---------------+
>   */
>  bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> -                       hwaddr *raddr, int *psizep, int *protp,
> +                       hwaddr *raddr, int *psizep, int *protp, int mmu_idx,
>                         bool guest_visible)
>  {
>      CPUPPCState *env = &cpu->env;
> @@ -474,17 +479,17 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>      hwaddr g_raddr;
>      bool relocation;
>  
> -    assert(!(msr_hv && cpu->vhyp));
> +    assert(!(mmuidx_hv(mmu_idx) && cpu->vhyp));
>  
> -    relocation = (access_type == MMU_INST_FETCH ? msr_ir : msr_dr);
> +    relocation = !mmuidx_real(mmu_idx);
>  
>      /* HV or virtual hypervisor Real Mode Access */
> -    if (!relocation && (msr_hv || cpu->vhyp)) {
> +    if (!relocation && (mmuidx_hv(mmu_idx) || cpu->vhyp)) {
>          /* In real mode top 4 effective addr bits (mostly) ignored */
>          *raddr = eaddr & 0x0FFFFFFFFFFFFFFFULL;
>  
>          /* In HV mode, add HRMOR if top EA bit is clear */
> -        if (msr_hv || !env->has_hv_mode) {
> +        if (mmuidx_hv(mmu_idx) || !env->has_hv_mode) {
>              if (!(eaddr >> 63)) {
>                  *raddr |= env->spr[SPR_HRMOR];
>             }
> @@ -546,7 +551,7 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>      if (relocation) {
>          int ret = ppc_radix64_process_scoped_xlate(cpu, access_type, eaddr, pid,
>                                                     pate, &g_raddr, &prot,
> -                                                   &psize, guest_visible);
> +                                                   &psize, mmu_idx, guest_visible);
>          if (ret) {
>              return false;
>          }
> @@ -564,13 +569,13 @@ bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>           * quadrants 1 or 2. Translates a guest real address to a host
>           * real address.
>           */
> -        if (lpid || !msr_hv) {
> +        if (lpid || !mmuidx_hv(mmu_idx)) {
>              int ret;
>  
>              ret = ppc_radix64_partition_scoped_xlate(cpu, access_type, eaddr,
>                                                       g_raddr, pate, raddr,
>                                                       &prot, &psize, false,
> -                                                     guest_visible);
> +                                                     mmu_idx, guest_visible);
>              if (ret) {
>                  return false;
>              }
> diff --git a/target/ppc/mmu-radix64.h b/target/ppc/mmu-radix64.h
> index 6b13b89b64..b70357cf34 100644
> --- a/target/ppc/mmu-radix64.h
> +++ b/target/ppc/mmu-radix64.h
> @@ -45,7 +45,7 @@
>  #ifdef TARGET_PPC64
>  
>  bool ppc_radix64_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
> -                       hwaddr *raddr, int *psizep, int *protp,
> +                       hwaddr *raddr, int *psizep, int *protp, int mmu_idx,
>                         bool guest_visible);
>  
>  static inline int ppc_radix64_get_prot_eaa(uint64_t pte)
> diff --git a/target/ppc/mmu_helper.c b/target/ppc/mmu_helper.c
> index ba1952c77d..9dcdf88597 100644
> --- a/target/ppc/mmu_helper.c
> +++ b/target/ppc/mmu_helper.c
> @@ -2908,7 +2908,7 @@ static bool ppc_xlate(PowerPCCPU *cpu, vaddr eaddr, MMUAccessType access_type,
>      case POWERPC_MMU_3_00:
>          if (ppc64_v3_radix(cpu)) {
>              return ppc_radix64_xlate(cpu, eaddr, access_type,
> -                                     raddrp, psizep, protp, guest_visible);
> +                                     raddrp, psizep, protp, mmu_idx, guest_visible);
>          }
>          /* fall through */
>      case POWERPC_MMU_64B:
> @@ -2941,8 +2941,10 @@ hwaddr ppc_cpu_get_phys_page_debug(CPUState *cs, vaddr addr)
>       * try an MMU_DATA_LOAD, we may not be able to read instructions
>       * mapped by code TLBs, so we also try a MMU_INST_FETCH.
>       */
> -    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p, 0, false) ||
> -        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p, 0, false)) {
> +    if (ppc_xlate(cpu, addr, MMU_DATA_LOAD, &raddr, &s, &p,
> +                  cpu_mmu_index(&cpu->env, false), false) ||
> +        ppc_xlate(cpu, addr, MMU_INST_FETCH, &raddr, &s, &p,
> +                  cpu_mmu_index(&cpu->env, true), false)) {
>          return raddr & TARGET_PAGE_MASK;
>      }
>      return -1;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

end of thread, other threads:[~2021-06-24  6:59 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-21 12:51 [PATCH v2 00/10] Clean up MMU translation Bruno Larsen (billionai)
2021-06-21 12:51 ` [PATCH v2 01/10] target/ppc: Remove PowerPCCPUClass.handle_mmu_fault Bruno Larsen (billionai)
2021-06-22 10:49   ` Greg Kurz
2021-06-24  1:40   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 02/10] target/ppc: Use MMUAccessType with *_handle_mmu_fault Bruno Larsen (billionai)
2021-06-22 12:05   ` Greg Kurz
2021-06-24  3:19   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 03/10] target/ppc: Push real-mode handling into ppc_radix64_xlate Bruno Larsen (billionai)
2021-06-24  3:29   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 04/10] target/ppc: Use bool success for ppc_radix64_xlate Bruno Larsen (billionai)
2021-06-24  3:31   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 05/10] target/ppc: Split out ppc_hash64_xlate Bruno Larsen (billionai)
2021-06-24  5:55   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 06/10] target/ppc: Split out ppc_hash32_xlate Bruno Larsen (billionai)
2021-06-21 12:51 ` [PATCH v2 07/10] target/ppc: Split out ppc_jumbo_xlate Bruno Larsen (billionai)
2021-06-24  6:30   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 08/10] target/ppc: Introduce ppc_xlate Bruno Larsen (billionai)
2021-06-24  6:34   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 09/10] target/ppc: Restrict ppc_cpu_tlb_fill to TCG Bruno Larsen (billionai)
2021-06-24  6:35   ` David Gibson
2021-06-21 12:51 ` [PATCH v2 10/10] target/ppc: fix address translation bug for radix mmus Bruno Larsen (billionai)
2021-06-24  6:48   ` David Gibson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).