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

[Scott Wood]

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.

BTW, PPC patches should be sent to both qemu-devel@ and qemu-ppc@, not
just the latter.

-Scott