qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: Alexander Graf
Subject: Re: [Qemu-devel] Minor MMU fixes for PowerPC 40x emulation
Date: Sat, 2 Oct 2010 19:10:03 +0200

On 02.10.2010, at 19:06, Edgar E. Iglesias wrote:

> On Sat, Oct 02, 2010 at 06:55:36PM +0200, Alexander Graf wrote:
>> 
>> On 02.10.2010, at 18:49, John Clark wrote:
>> 
>>>>>           /* 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.
>> 
>> The ppc emulation code lacks a lot of defines. In fact, the same goes for 
>> x86 emulation too ;). But that doesn't mean we have to keep it that way!
>> 
>>> 
>>>>>   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:
>> 
>> Well the thing that strikes me as weird is mostly that you're changing a 
>> function name, but no callers to it. So is this function never used? Or was 
>> tlbre_lo defined before already and is now defined twice?
> 
> Hi,
> 
> Alex:
> I think you've missed the part of the patch that renames the _lo -> _hi.
> As John says, qemu had the hi/lo parts reversed when reading 4xx TLB regs.

Oh. THERE it is! Hah. Yeah, I really missed that line - exchanging both 
functions makes sense.


Thanks,

Alex




reply via email to

[Prev in Thread] Current Thread [Next in Thread]