linux-mips
[Top] [All Lists]

Re: [PATCH 1/2] MIPS: Replace some magic numbers with symbolic values in

To: David Daney <ddaney@caviumnetworks.com>
Subject: Re: [PATCH 1/2] MIPS: Replace some magic numbers with symbolic values in tlbex.c
From: Paul Gortmaker <paul.gortmaker@windriver.com>
Date: Wed, 13 May 2009 17:34:55 -0400
Cc: linux-mips@linux-mips.org, ralf@linux-mips.org, David VomLehn <dvomlehn@cisco.com>
In-reply-to: <1242247717-21324-1-git-send-email-ddaney@caviumnetworks.com>
Original-recipient: rfc822;linux-mips@linux-mips.org
References: <4A0B31C6.9050804@caviumnetworks.com> <1242247717-21324-1-git-send-email-ddaney@caviumnetworks.com>
Sender: linux-mips-bounce@linux-mips.org
User-agent: Mutt/1.5.18 (2008-05-17)
[[PATCH 1/2] MIPS: Replace some magic numbers with symbolic values in tlbex.c] 
On 13/05/2009 (Wed 13:48) David Daney wrote:

> The logic used to split the r4000 refill handler is liberally
> sprinkled with magic numbers.  We attempt to explain what they are and
> normalize them against a new symbolic value (MIPS64_REFILL_INSNS).
> 
> CC: Paul Gortmaker <paul.gortmaker@windriver.com>
> CC: David VomLehn <dvomlehn@cisco.com>
> Signed-off-by: David Daney <ddaney@caviumnetworks.com>
> ---
>  arch/mips/mm/tlbex.c |   34 ++++++++++++++++++++++++++--------
>  1 files changed, 26 insertions(+), 8 deletions(-)
> 
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index 4dc4f3e..d99ed78 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -649,6 +649,14 @@ static void __cpuinit build_update_entries(u32 **p, 
> unsigned int tmp,
>  #endif
>  }
>  
> +/*
> + * For a 64-bit kernel, we are using the 64-bit XTLB refill execption

                                              "exception"

 -- aside from that, I've double checked that the changes should be inert
(excluding the improved readability).   You can add a:

Reviewed-by: Paul Gortmaker <paul.gortmaker@windriver.com>

if you want, or given that it really isn't a complex patch, feel free
to reduce the clutter and no need to mention me at all.  Your choice.

Thanks for the cleanup,
Paul.

> + * because EXL == 0.  If we wrap, we can also use the 32 instruction
> + * slots before the XTLB refill exception handler which belong to the
> + * unused TLB refill exception.
> + */
> +#define MIPS64_REFILL_INSNS 32
> +
>  static void __cpuinit build_r4000_tlb_refill_handler(void)
>  {
>       u32 *p = tlb_handler;
> @@ -702,9 +710,10 @@ static void __cpuinit 
> build_r4000_tlb_refill_handler(void)
>       if ((p - tlb_handler) > 64)
>               panic("TLB refill handler space exceeded");
>  #else
> -     if (((p - tlb_handler) > 63)
> -         || (((p - tlb_handler) > 61)
> -             && uasm_insn_has_bdelay(relocs, tlb_handler + 29)))
> +     if (((p - tlb_handler) > (MIPS64_REFILL_INSNS * 2) - 1)
> +         || (((p - tlb_handler) > (MIPS64_REFILL_INSNS * 2) - 3)
> +             && uasm_insn_has_bdelay(relocs,
> +                                     tlb_handler + MIPS64_REFILL_INSNS - 3)))
>               panic("TLB refill handler space exceeded");
>  #endif
>  
> @@ -717,16 +726,24 @@ static void __cpuinit 
> build_r4000_tlb_refill_handler(void)
>       uasm_copy_handler(relocs, labels, tlb_handler, p, f);
>       final_len = p - tlb_handler;
>  #else /* CONFIG_64BIT */
> -     f = final_handler + 32;
> -     if ((p - tlb_handler) <= 32) {
> +     f = final_handler + MIPS64_REFILL_INSNS;
> +     if ((p - tlb_handler) <= MIPS64_REFILL_INSNS) {
>               /* Just copy the handler. */
>               uasm_copy_handler(relocs, labels, tlb_handler, p, f);
>               final_len = p - tlb_handler;
>       } else {
> -             u32 *split = tlb_handler + 30;
> +             /*
> +              * Split two instructions before the end.  One for the
> +              * branch and one for the instruction in the delay
> +              * slot.
> +              */
> +             u32 *split = tlb_handler + MIPS64_REFILL_INSNS - 2;
>  
>               /*
> -              * Find the split point.
> +              * Find the split point.  If the branch would fall in
> +              * a delay slot, we must back up an additional
> +              * instruction so that it is no longer in a delay
> +              * slot.
>                */
>               if (uasm_insn_has_bdelay(relocs, split - 1))
>                       split--;
> @@ -749,7 +766,8 @@ static void __cpuinit build_r4000_tlb_refill_handler(void)
>  
>               /* Copy the rest of the handler. */
>               uasm_copy_handler(relocs, labels, split, p, final_handler);
> -             final_len = (f - (final_handler + 32)) + (p - split);
> +             final_len = (f - (final_handler + MIPS64_REFILL_INSNS)) +
> +                         (p - split);
>       }
>  #endif /* CONFIG_64BIT */
>  
> -- 
> 1.6.0.6
> 

<Prev in Thread] Current Thread [Next in Thread>