Re: [Qemu-ppc] [PATCH 2/2] PPC: Fix TLB invalidation bug within the PPC interrupt handler.

[David Gibson]

On Thu, Mar 22, 2012 at 03:37:49PM -0500, Scott Wood wrote:
> On 03/22/2012 01:57 PM, Mark Cave-Ayland wrote:
> > Commit 41557447d30eeb944e42069513df13585f5e6c7f also introduced a subtle TLB
> > flush bug. By applying a mask to the interrupt MSR which cleared the IR/DR
> > bits at the start of the interrupt handler, the logic towards the end of the
> > handler to force a TLB flush if either one of these bits were set would 
> > never
> > be triggered.
> > 
> > This patch simply changes the IR/DR bit check in the TLB flush logic to use
> > the original MSR value (albeit with some interrupt-specific bits cleared) so
> > that the IR/DR bits are preserved at the point where the check takes place.
> > 
> > Signed-off-by: Mark Cave-Ayland <address@hidden>
> > ---
> >  target-ppc/helper.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> > 
> > diff --git a/target-ppc/helper.c b/target-ppc/helper.c
> > index 4a21d8d..5042c28 100644
> > --- a/target-ppc/helper.c
> > +++ b/target-ppc/helper.c
> > @@ -2964,7 +2964,7 @@ static inline void powerpc_excp(CPUPPCState *env, int 
> > excp_model, int excp)
> >      if (asrr1 != -1)
> >          env->spr[asrr1] = env->spr[srr1];
> >      /* If we disactivated any translation, flush TLBs */
> > -    if (new_msr & ((1 << MSR_IR) | (1 << MSR_DR)))
> > +    if (msr & ((1 << MSR_IR) | (1 << MSR_DR)))
> >          tlb_flush(env, 1);
> >  
> >      if (msr_ile) {
> 
> I wonder why this is done again at the end of the function for booke
> (without regard to MSR bits).  It seems like the above flush should
> handle booke as well as classic -- though the comment should be
> "changed/deactivated" rather than "disactivated", since on booke those
> bits just switch from one translation to another.

Right, which means I don't think this test will work as is for BookE.
There, we'd need to check for any change in the IS/DS bits instead of
just testing presence of IR/DR bits.

-- 
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