On 06/23/2016 04:01 PM, alar...@ddci.com wrote:
> On 17 Jun 2016 22:55:47 Scott Wood wrote:
> On 06/17/2016 05:13 PM, Aaron Larson wrote:
>
> AL> The root cause is that the function computing the size of the TLB
> AL> entry, namely booke206_page_size_to_tlb ... [is wrong]
> AL> --- a/hw/ppc/ppce500_spin.c
> AL> +++ b/hw/ppc/ppce500_spin.c
> AL> @@ -75,7 +75,11 @@ static void spin_reset(void *opaque)
> AL> /* Create -kernel TLB entries for BookE, linearly spanning 256MB. */
> AL> static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
> AL> {
> AL> - return ctz32(size >> 10) >> 1;
> AL> ...
> AL> + return ctz32(size >> 10) >> (MAS1_TSIZE_SHIFT - 7);
>
> SW> The patch that changed MAS1_TSIZE_SHIFT from 8 to 7 was around the
> SW> same time as the patch that added this code, which is probably why
> SW> adjusting it got missed. Commit 2bd9543cd3 did update the
> SW> equivalent code in ppce500_mpc8544ds.c, which now resides in
> SW> hw/ppc/e500.c and has been changed to not assume a power-of-2
> SW> size. The ppce500_spin version should be eliminated.
>
> Scott, Are you suggesting that the e500.c:booke206_page_size_to_tlb()
> function be made out of line and exported so that it can be used by
> ppce500_spin.c? If so I'll submit a patch post haste.
Yes, that's what I'm suggesting.
> I am uncertain though because that still leaves a lot of similar, but
> still different code between ppce500_spin.c and e500.c.
The potential for additional cleanup doesn't make this particular
cleanup wrong.
-Scott
