| Message ID | 1284214197-27140-2-git-send-email-edgar.iglesias@gmail.com |
|---|---|
| State | New |
| Headers | show |
Am 11.09.2010 um 16:09 schrieb Edgar E. Iglesias: > Improve the emulation of the BookE MMU to be able to boot linux > on virtex5 boards. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com> > --- > target-ppc/helper.c | 46 +++++++++++++++++++++++++++++++++++++++ > +------ > 1 files changed, 40 insertions(+), 6 deletions(-) > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index a7ec1f4..4c810d1 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c [...] > @@ -1445,7 +1452,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, > target_ulong address, int rw, > break; > case POWERPC_MMU_BOOKE: > /* XXX: TODO */ Is this TODO still valid now? If yes, could you please fill in what needs to be done there. > - cpu_abort(env, "BookE MMU model is not > implemented\n"); > + env->exception_index = POWERPC_EXCP_ITLB; > + env->error_code = 0; > + env->spr[SPR_BOOKE_DEAR] = address; > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > return -1; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ [...] > @@ -1557,7 +1571,14 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, > target_ulong address, int rw, > break; > case POWERPC_MMU_BOOKE: > /* XXX: TODO */ Same here. > - cpu_abort(env, "BookE MMU model is not > implemented\n"); > + env->exception_index = POWERPC_EXCP_DTLB; > + env->error_code = 0; > + env->spr[SPR_BOOKE_DEAR] = address; > + if (rw) { > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > + } else { > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > + } > return -1; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ [...] > @@ -1848,8 +1876,7 @@ void ppc_tlb_invalidate_all (CPUPPCState *env) > cpu_abort(env, "MPC8xx MMU model is not implemented\n"); > break; > case POWERPC_MMU_BOOKE: > - /* XXX: TODO */ Here you removed it, for comparison. Andreas > - cpu_abort(env, "BookE MMU model is not implemented\n"); > + tlb_flush(env, 1); > break; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ [snip]
On Sat, Sep 11, 2010 at 04:42:19PM +0200, Andreas Färber wrote: > Am 11.09.2010 um 16:09 schrieb Edgar E. Iglesias: > > > Improve the emulation of the BookE MMU to be able to boot linux > > on virtex5 boards. > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com> > > --- > > target-ppc/helper.c | 46 +++++++++++++++++++++++++++++++++++++++ > > +------ > > 1 files changed, 40 insertions(+), 6 deletions(-) > > > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > > index a7ec1f4..4c810d1 100644 > > --- a/target-ppc/helper.c > > +++ b/target-ppc/helper.c > [...] > > @@ -1445,7 +1452,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, > > target_ulong address, int rw, > > break; > > case POWERPC_MMU_BOOKE: > > /* XXX: TODO */ > > Is this TODO still valid now? If yes, could you please fill in what > needs to be done there. > > > - cpu_abort(env, "BookE MMU model is not > > implemented\n"); > > + env->exception_index = POWERPC_EXCP_ITLB; > > + env->error_code = 0; > > + env->spr[SPR_BOOKE_DEAR] = address; > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > return -1; > > case POWERPC_MMU_BOOKE_FSL: > > /* XXX: TODO */ > [...] > > @@ -1557,7 +1571,14 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, > > target_ulong address, int rw, > > break; > > case POWERPC_MMU_BOOKE: > > /* XXX: TODO */ > > Same here. > > > - cpu_abort(env, "BookE MMU model is not > > implemented\n"); > > + env->exception_index = POWERPC_EXCP_DTLB; > > + env->error_code = 0; > > + env->spr[SPR_BOOKE_DEAR] = address; > > + if (rw) { > > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > > + } else { > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > + } > > return -1; > > case POWERPC_MMU_BOOKE_FSL: > > /* XXX: TODO */ > [...] > > @@ -1848,8 +1876,7 @@ void ppc_tlb_invalidate_all (CPUPPCState *env) > > cpu_abort(env, "MPC8xx MMU model is not implemented\n"); > > break; > > case POWERPC_MMU_BOOKE: > > - /* XXX: TODO */ > > Here you removed it, for comparison. OK, I've removed the XXX comments. Thanks
Edgar E. Iglesias wrote: > Improve the emulation of the BookE MMU to be able to boot linux > on virtex5 boards. > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com> > --- > target-ppc/helper.c | 46 ++++++++++++++++++++++++++++++++++++++++------ > 1 files changed, 40 insertions(+), 6 deletions(-) > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > index a7ec1f4..4c810d1 100644 > --- a/target-ppc/helper.c > +++ b/target-ppc/helper.c > @@ -1325,8 +1325,15 @@ int get_physical_address (CPUState *env, mmu_ctx_t *ctx, target_ulong eaddr, > #endif > if ((access_type == ACCESS_CODE && msr_ir == 0) || > (access_type != ACCESS_CODE && msr_dr == 0)) { > - /* No address translation */ > - ret = check_physical(env, ctx, eaddr, rw); > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > + /* The BookE MMU always performs address translation. The > + IS and DS bits only affect the address space. */ > + ret = mmubooke_get_physical_address(env, ctx, eaddr, > + rw, access_type); > + } else { > + /* No address translation. */ > + ret = check_physical(env, ctx, eaddr, rw); > + } > } else { > ret = -1; > switch (env->mmu_model) { > @@ -1445,7 +1452,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > break; > case POWERPC_MMU_BOOKE: > /* XXX: TODO */ > - cpu_abort(env, "BookE MMU model is not implemented\n"); > + env->exception_index = POWERPC_EXCP_ITLB; > + env->error_code = 0; > + env->spr[SPR_BOOKE_DEAR] = address; > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > Uh - I don't see ESR be modified here in the spec. > return -1; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ > @@ -1471,6 +1481,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > break; > case -3: > /* No execute protection violation */ > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > + env->spr[SPR_BOOKE_DEAR] = address; > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > + } > env->exception_index = POWERPC_EXCP_ISI; > ISIs don't set DEAR. Only data exceptions do. > env->error_code = 0x10000000; > Hrm. What is error_code? It's basically what later on becomes ESR, no? Shouldn't we rather have the error_code setter here be conditional and later on set ESR to error_code? SRR1 on BookE is unaffected by error codes. > break; > @@ -1557,7 +1571,14 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > break; > case POWERPC_MMU_BOOKE: > /* XXX: TODO */ > - cpu_abort(env, "BookE MMU model is not implemented\n"); > + env->exception_index = POWERPC_EXCP_DTLB; > + env->error_code = 0; > + env->spr[SPR_BOOKE_DEAR] = address; > + if (rw) { > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > I would really prefer a #define for ESR_ST. By then you can also just do env->spr[SPR_BOOKE_ESR] = rw ? 0 : ESR_ST; That's more readable IMHO and doesn't clutter the code with braces. > + } else { > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > + } > return -1; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ > @@ -1582,6 +1603,13 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > if (rw) { > env->spr[SPR_40x_ESR] |= 0x00800000; > } > + } else if (env->mmu_model == POWERPC_MMU_BOOKE) { > + env->spr[SPR_BOOKE_DEAR] = address; > + if (rw) { > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > + } else { > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > + } > } else { > env->spr[SPR_DAR] = address; > if (rw == 1) { > @@ -1848,8 +1876,7 @@ void ppc_tlb_invalidate_all (CPUPPCState *env) > cpu_abort(env, "MPC8xx MMU model is not implemented\n"); > break; > case POWERPC_MMU_BOOKE: > - /* XXX: TODO */ > - cpu_abort(env, "BookE MMU model is not implemented\n"); > + tlb_flush(env, 1); > Shouldn't this also clear the entries from the TLB? tlb_flush only flushes the qemu TLB, no? > break; > case POWERPC_MMU_BOOKE_FSL: > /* XXX: TODO */ > @@ -2629,6 +2656,13 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) > /* Reset exception state */ > env->exception_index = POWERPC_EXCP_NONE; > env->error_code = 0; > + > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > + /* XXX: The BookE changes address space when switching modes, > + we should probably implement that as different MMU indexes, > + but for the moment we do it the slow way and flush all. */ > + tlb_flush(env, 1); > Ugh. Yeah, the Qemu internal TLB really needs some work to fit PPC well. Alex
On Mon, Sep 20, 2010 at 12:35:02PM +0200, Alexander Graf wrote: > Edgar E. Iglesias wrote: > > Improve the emulation of the BookE MMU to be able to boot linux > > on virtex5 boards. > > > > Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com> > > --- > > target-ppc/helper.c | 46 ++++++++++++++++++++++++++++++++++++++++------ > > 1 files changed, 40 insertions(+), 6 deletions(-) > > > > diff --git a/target-ppc/helper.c b/target-ppc/helper.c > > index a7ec1f4..4c810d1 100644 > > --- a/target-ppc/helper.c > > +++ b/target-ppc/helper.c > > @@ -1325,8 +1325,15 @@ int get_physical_address (CPUState *env, mmu_ctx_t *ctx, target_ulong eaddr, > > #endif > > if ((access_type == ACCESS_CODE && msr_ir == 0) || > > (access_type != ACCESS_CODE && msr_dr == 0)) { > > - /* No address translation */ > > - ret = check_physical(env, ctx, eaddr, rw); > > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > > + /* The BookE MMU always performs address translation. The > > + IS and DS bits only affect the address space. */ > > + ret = mmubooke_get_physical_address(env, ctx, eaddr, > > + rw, access_type); > > + } else { > > + /* No address translation. */ > > + ret = check_physical(env, ctx, eaddr, rw); > > + } > > } else { > > ret = -1; > > switch (env->mmu_model) { > > @@ -1445,7 +1452,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > > break; > > case POWERPC_MMU_BOOKE: > > /* XXX: TODO */ > > - cpu_abort(env, "BookE MMU model is not implemented\n"); > > + env->exception_index = POWERPC_EXCP_ITLB; > > + env->error_code = 0; > > + env->spr[SPR_BOOKE_DEAR] = address; > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > > > Uh - I don't see ESR be modified here in the spec. Right, will fix that. > > > return -1; > > case POWERPC_MMU_BOOKE_FSL: > > /* XXX: TODO */ > > @@ -1471,6 +1481,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > > break; > > case -3: > > /* No execute protection violation */ > > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > > + env->spr[SPR_BOOKE_DEAR] = address; > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > + } > > env->exception_index = POWERPC_EXCP_ISI; > > > > ISIs don't set DEAR. Only data exceptions do. OK, will fix that. > > > env->error_code = 0x10000000; > > > > Hrm. What is error_code? It's basically what later on becomes ESR, no? > Shouldn't we rather have the error_code setter here be conditional and > later on set ESR to error_code? SRR1 on BookE is unaffected by error codes. It looks suspicious, but I didn't add that logic so I think it's fair to leave that kind of investigation as future improvments. > > > break; > > @@ -1557,7 +1571,14 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > > break; > > case POWERPC_MMU_BOOKE: > > /* XXX: TODO */ > > - cpu_abort(env, "BookE MMU model is not implemented\n"); > > + env->exception_index = POWERPC_EXCP_DTLB; > > + env->error_code = 0; > > + env->spr[SPR_BOOKE_DEAR] = address; > > + if (rw) { > > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > > > > I would really prefer a #define for ESR_ST. By then you can also just do > > env->spr[SPR_BOOKE_ESR] = rw ? 0 : ESR_ST; > > That's more readable IMHO and doesn't clutter the code with braces. Sounds good. > > > + } else { > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > + } > > return -1; > > case POWERPC_MMU_BOOKE_FSL: > > /* XXX: TODO */ > > @@ -1582,6 +1603,13 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, > > if (rw) { > > env->spr[SPR_40x_ESR] |= 0x00800000; > > } > > + } else if (env->mmu_model == POWERPC_MMU_BOOKE) { > > + env->spr[SPR_BOOKE_DEAR] = address; > > + if (rw) { > > + env->spr[SPR_BOOKE_ESR] = 0x00800000; > > + } else { > > + env->spr[SPR_BOOKE_ESR] = 0x00000000; > > + } > > } else { > > env->spr[SPR_DAR] = address; > > if (rw == 1) { > > @@ -1848,8 +1876,7 @@ void ppc_tlb_invalidate_all (CPUPPCState *env) > > cpu_abort(env, "MPC8xx MMU model is not implemented\n"); > > break; > > case POWERPC_MMU_BOOKE: > > - /* XXX: TODO */ > > - cpu_abort(env, "BookE MMU model is not implemented\n"); > > + tlb_flush(env, 1); > > > > Shouldn't this also clear the entries from the TLB? tlb_flush only > flushes the qemu TLB, no? No, these helper calls are only for clearing the internal tables AFAICT. > > > break; > > case POWERPC_MMU_BOOKE_FSL: > > /* XXX: TODO */ > > @@ -2629,6 +2656,13 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) > > /* Reset exception state */ > > env->exception_index = POWERPC_EXCP_NONE; > > env->error_code = 0; > > + > > + if (env->mmu_model == POWERPC_MMU_BOOKE) { > > + /* XXX: The BookE changes address space when switching modes, > > + we should probably implement that as different MMU indexes, > > + but for the moment we do it the slow way and flush all. */ > > + tlb_flush(env, 1); > > > > Ugh. Yeah, the Qemu internal TLB really needs some work to fit PPC well. Well, the problem is not really with the internal qemu tlb, but more with how we use it in the booke emulation (i.e lazyness from my side). Thanks for the good review. Cheers
diff --git a/target-ppc/helper.c b/target-ppc/helper.c index a7ec1f4..4c810d1 100644 --- a/target-ppc/helper.c +++ b/target-ppc/helper.c @@ -1325,8 +1325,15 @@ int get_physical_address (CPUState *env, mmu_ctx_t *ctx, target_ulong eaddr, #endif if ((access_type == ACCESS_CODE && msr_ir == 0) || (access_type != ACCESS_CODE && msr_dr == 0)) { - /* No address translation */ - ret = check_physical(env, ctx, eaddr, rw); + if (env->mmu_model == POWERPC_MMU_BOOKE) { + /* The BookE MMU always performs address translation. The + IS and DS bits only affect the address space. */ + ret = mmubooke_get_physical_address(env, ctx, eaddr, + rw, access_type); + } else { + /* No address translation. */ + ret = check_physical(env, ctx, eaddr, rw); + } } else { ret = -1; switch (env->mmu_model) { @@ -1445,7 +1452,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, break; case POWERPC_MMU_BOOKE: /* XXX: TODO */ - cpu_abort(env, "BookE MMU model is not implemented\n"); + env->exception_index = POWERPC_EXCP_ITLB; + env->error_code = 0; + env->spr[SPR_BOOKE_DEAR] = address; + env->spr[SPR_BOOKE_ESR] = 0x00000000; return -1; case POWERPC_MMU_BOOKE_FSL: /* XXX: TODO */ @@ -1471,6 +1481,10 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, break; case -3: /* No execute protection violation */ + if (env->mmu_model == POWERPC_MMU_BOOKE) { + env->spr[SPR_BOOKE_DEAR] = address; + env->spr[SPR_BOOKE_ESR] = 0x00000000; + } env->exception_index = POWERPC_EXCP_ISI; env->error_code = 0x10000000; break; @@ -1557,7 +1571,14 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, break; case POWERPC_MMU_BOOKE: /* XXX: TODO */ - cpu_abort(env, "BookE MMU model is not implemented\n"); + env->exception_index = POWERPC_EXCP_DTLB; + env->error_code = 0; + env->spr[SPR_BOOKE_DEAR] = address; + if (rw) { + env->spr[SPR_BOOKE_ESR] = 0x00800000; + } else { + env->spr[SPR_BOOKE_ESR] = 0x00000000; + } return -1; case POWERPC_MMU_BOOKE_FSL: /* XXX: TODO */ @@ -1582,6 +1603,13 @@ int cpu_ppc_handle_mmu_fault (CPUState *env, target_ulong address, int rw, if (rw) { env->spr[SPR_40x_ESR] |= 0x00800000; } + } else if (env->mmu_model == POWERPC_MMU_BOOKE) { + env->spr[SPR_BOOKE_DEAR] = address; + if (rw) { + env->spr[SPR_BOOKE_ESR] = 0x00800000; + } else { + env->spr[SPR_BOOKE_ESR] = 0x00000000; + } } else { env->spr[SPR_DAR] = address; if (rw == 1) { @@ -1848,8 +1876,7 @@ void ppc_tlb_invalidate_all (CPUPPCState *env) cpu_abort(env, "MPC8xx MMU model is not implemented\n"); break; case POWERPC_MMU_BOOKE: - /* XXX: TODO */ - cpu_abort(env, "BookE MMU model is not implemented\n"); + tlb_flush(env, 1); break; case POWERPC_MMU_BOOKE_FSL: /* XXX: TODO */ @@ -2629,6 +2656,13 @@ static inline void powerpc_excp(CPUState *env, int excp_model, int excp) /* Reset exception state */ env->exception_index = POWERPC_EXCP_NONE; env->error_code = 0; + + if (env->mmu_model == POWERPC_MMU_BOOKE) { + /* XXX: The BookE changes address space when switching modes, + we should probably implement that as different MMU indexes, + but for the moment we do it the slow way and flush all. */ + tlb_flush(env, 1); + } } void do_interrupt (CPUState *env)
Improve the emulation of the BookE MMU to be able to boot linux on virtex5 boards. Signed-off-by: Edgar E. Iglesias <edgar.iglesias@gmail.com> --- target-ppc/helper.c | 46 ++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 40 insertions(+), 6 deletions(-)