Re: [Qemu-devel] Minor MMU fixes for PowerPC 40x emulation

[John Clark <clarkjc@runbox.com>]

>>             /* Check from TLB entry */
>> -            /* XXX: there is a problem here or in the TLB fill code... */
>> +            /* There is no longer a need to force PAGE_EXEC permission here */
>> +            /* because of the tlb->attr fix in helper_4xx_tlbwe_lo() */
> 
> I guess that comment is superfluous, as readers several years from now don't care what was broken back in the day :).

Yes, I suppose so :)

>> @@ -3939,7 +3939,7 @@ target_ulong helper_4xx_tlbre_lo (target_ulong entry)
>>     tlb = &env->tlb[entry].tlbe;
>>     ret = tlb->EPN;
>>     if (tlb->prot & PAGE_VALID)
>> -        ret |= 0x400;
>> +        ret |= 0x40;    /* V bit is 0x40, not 0x400 */
> 
> Ouch. Mind to make it a define?

Sure, I was surprised that there wasn't a define for that when I found it.

>>     size = booke_page_size_to_tlb(tlb->size);
>>     if (size < 0 || size > 0x7)
>>         size = 1;
>> @@ -3948,7 +3948,7 @@ target_ulong helper_4xx_tlbre_lo (target_ulong entry)
>>     return ret;
>> }
>>
>> -target_ulong helper_4xx_tlbre_hi (target_ulong entry)
>> +target_ulong helper_4xx_tlbre_lo (target_ulong entry)
> 
> Huh?

To summarize, 'tlbre' has two forms: one to retrieve the high bits of
a TLB entry (TLBHI), and one to retrieve the low bits (TLBLO) of a TLB
entry.  This code had the TLBLO form returning the bits corresponding
to TLBHI and vice versa, hence the name change.  You can verify this
if you like with this IBM PowerPC 405 core user manual on page 362:

https://www-01.ibm.com/chips/techlib/techlib.nsf/techdocs/D060DB54BD4DC4F2872569D2004A30D6/$file/ppc405fx_um.pdf

Thanks.

- John