Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size

[alarson]

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.

I am uncertain though because that still leaves a lot of similar, but
still different code between ppce500_spin.c and e500.c.

Re: [Qemu-devel] [Qemu-ppc] [PATCH] target-ppc: Correct ppc3500_spin initial TLB size

[Scott Wood]

On 06/23/2016 04:01 PM, alarson@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