[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-ppc] [PATCH 8/8] target-ppc Disentangle ppc64 hash mmu path fo
From: |
David Gibson |
Subject: |
Re: [Qemu-ppc] [PATCH 8/8] target-ppc Disentangle ppc64 hash mmu path for cpu_ppc_handle_mmu_fault |
Date: |
Sat, 23 Feb 2013 19:13:54 +1100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
On Fri, Feb 22, 2013 at 05:13:56PM +0100, Alexander Graf wrote:
> On 12.02.2013, at 03:00, David Gibson wrote:
[snip]
> > + } else if (ret < 0) {
> > + LOG_MMU_STATE(env);
> > + if (access_type == ACCESS_CODE) {
> > + switch (ret) {
> > + case -1:
> > + env->exception_index = POWERPC_EXCP_ISI;
> > + env->error_code = 0x40000000;
> > + break;
> > + case -2:
> > + /* Access rights violation */
>
> These really should become an enum or defines. Something where the
> thing we check on tells us what we're checking for without a comment
> :).
So, I didn't want to introduce an emum or whatever at the same time as
I was just moving the code around - makes for more chance of
mistakes. And by the end of the series, I've gotten rid of these
magic error values anyway, by reporting the exceptions in-line in mmu
fault handle, so I think it's kind of moot.
[snip]
> > + }
> > + }
> > +#if 0
>
> #ifdef DEBUG_MMU?
Again, this is just moved code.
> > + printf("%s: set exception to %d %02x\n", __func__,
> > + env->exception, env->error_code);
> > +#endif
> > + ret = 1;
> > + }
> > +
> > + return ret;
> > +}
> > diff --git a/target-ppc/mmu_helper.c b/target-ppc/mmu_helper.c
> > index 98143dd..f8f213b 100644
> > --- a/target-ppc/mmu_helper.c
> > +++ b/target-ppc/mmu_helper.c
> > @@ -1533,6 +1533,18 @@ int cpu_ppc_handle_mmu_fault(CPUPPCState *env,
> > target_ulong address, int rw,
> > int access_type;
> > int ret = 0;
> >
> > + switch (env->mmu_model) {
> > +#if defined(TARGET_PPC64)
> > + case POWERPC_MMU_64B:
> > + case POWERPC_MMU_2_06:
> > + case POWERPC_MMU_2_06d:
> > + return ppc_hash64_handle_mmu_fault(env, address, rw, mmu_idx);
> > +#endif
> > +
> > + default:
> > + ; /* Otherwise fall through to the general code below */
> > + }
>
> Sigh. This too should become a class function. Maybe it is within
> the scope of this patch set after all :). You could keep the
> "legacy" handling code alive and just point the non-converted
> targets to the existing function. Then override the handler for
> book3s_64 families to ppc_hash64_handle_mmu_fault.
Well, as I said, I think it makes sense to move the switch to one
place first, then change it to a method dispatch. I can look at
adding patches to do that conversion to the end of the series, but
I'll need to pull down the cpu qoming stuff furst.
--
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
signature.asc
Description: Digital signature
[Qemu-ppc] [PATCH 2/8] target-ppc: Move SLB handling into a mmu-hash64.c, David Gibson, 2013/02/11
[Qemu-ppc] [PATCH 4/8] target-ppc: Disentangle 64-bit version of find_pte(), David Gibson, 2013/02/11