[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [RFC PATCH] target/ppc: fix address translation bug for hash table m

From: Richard Henderson
Subject: Re: [RFC PATCH] target/ppc: fix address translation bug for hash table mmus
Date: Wed, 2 Jun 2021 12:26:38 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.8.1

On 6/2/21 12:18 PM, Bruno Larsen (billionai) wrote:
Based-on: <20210518201146.794854-1-richard.henderson@linaro.org>

This commit attempts to implement a first draft of a solution to the
first bug mentioned by Richard Henderson in this e-mail
The second bug was not touched, which is basically implementing the
solution C

To sumarize the first bug here, from my understanding, when an address
translation is asked of a 64bit mmu that uses hashtables, the code
attempts to check some permission bits, but checks them from the wrong

The solution implemented here is more complex than necessary on
purpose, to make it more readable (and make sure I understand what is
going on). If that would really fix the problem, I'll move to
implementing an actual solution, and to all affected functions.

Signed-off-by: Bruno Larsen (billionai) <bruno.larsen@eldorado.org.br>
  target/ppc/mmu-hash64.c | 12 ++++++++++--
  1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/target/ppc/mmu-hash64.c b/target/ppc/mmu-hash64.c
index c1b98a97e9..63f10f1be7 100644
--- a/target/ppc/mmu-hash64.c
+++ b/target/ppc/mmu-hash64.c
@@ -887,6 +887,14 @@ bool ppc_hash64_xlate(PowerPCCPU *cpu, vaddr eaddr, 
MMUAccessType access_type,
      int exec_prot, pp_prot, amr_prot, prot;
      int need_prot;
      hwaddr raddr;
+    unsigned immu_idx, dmmu_idx;
+    immu_idx = (env->hflags >> HFLAGS_IMMU_IDX) & 7;
+    dmmu_idx = (env->hflags >> HFLAGS_DMMU_IDX) & 7;

This doesn't help at all with the reported bug. You're still reading from env. You need the mmu_idx that was passed to ppc_cpu_tlb_fill.

For the use from ppc_cpu_get_phys_page_debug, you'd pass in cpu_mmu_index(env, false).

+    const short HV = 1, IR = 2, DR = 3;
+    bool MSR[3];
+    MSR[HV] = dmmu_idx & 2,
+    MSR[IR] = immu_idx & 4,
+    MSR[DR] = dmmu_idx & 4;

There's no point in the array. Just use three different scalars (real_mode, hv, and pr (note that pr is the major portion of the bug as reported)). Additionally, you'll not be distinguishing immu_idx and dmmu_idx, but using the single idx that's given.

-    if (access_type == MMU_INST_FETCH ? !msr_ir : !msr_dr) {
+    if (access_type == MMU_INST_FETCH ? !MSR[IR] : !MSR[DR]) {

Which simplifies this condition to just a single test.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]