Re: [Qemu-devel] [Qemu-ppc] [PATCH] booke_206_tlbwe: Discard invalid bits in MAS2

[Fabien Chouteau <chouteau@adacore.com>]

On 05/21/2012 03:57 PM, Alexander Graf wrote:
> On 05/21/2012 03:47 PM, Fabien Chouteau wrote:
>> On 05/21/2012 01:11 PM, Alexander Graf wrote:
>>> On 05/21/2012 12:59 PM, Fabien Chouteau wrote:
>>>> On 05/21/2012 11:08 AM, Alexander Graf wrote:
>>>>> On 21.05.2012, at 10:56, Fabien Chouteau wrote:
>>>>>
>>>>>> On 05/20/2012 12:18 PM, Alexander Graf wrote:
>>>>>>> On 20.05.2012, at 12:15, Alexander Graf<agraf@suse.de>   wrote:
>>>>>>>
>>>>>>>> On 09.05.2012, at 15:28, Fabien Chouteau<chouteau@adacore.com>   wrote:
>>>>>>>>
>>>>>>>>> The size of EPN field in MAS2 depends on page size. This patch adds a
>>>>>>>>> mask to discard invalid bits in EPN field.
>>>>>>>>>
>>>>>>>>> Definition of EPN field from e500v2 RM:
>>>>>>>>> EPN Effective page number: Depending on page size, only the bits
>>>>>>>>> associated with a page boundary are valid. Bits that represent offsets
>>>>>>>>> within a page are ignored and should be cleared.
>>>>>>>>>
>>>>>>>>> There is a similar (but more complicated) definition in PowerISA V2.06.
>>>>>>>>>
>>>>>>>>> Signed-off-by: Fabien Chouteau<chouteau@adacore.com>
>>>>>>>>> ---
>>>>>>>>> target-ppc/op_helper.c |   10 ++++++++--
>>>>>>>>> 1 file changed, 8 insertions(+), 2 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
>>>>>>>>> index 4ef2332..6bc64ad 100644
>>>>>>>>> --- a/target-ppc/op_helper.c
>>>>>>>>> +++ b/target-ppc/op_helper.c
>>>>>>>>> @@ -4227,6 +4227,8 @@ void helper_booke206_tlbwe(void)
>>>>>>>>>     uint32_t tlbncfg, tlbn;
>>>>>>>>>     ppcmas_tlb_t *tlb;
>>>>>>>>>     uint32_t size_tlb, size_ps;
>>>>>>>>> +    target_ulong mask;
>>>>>>>>> +
>>>>>>>>>
>>>>>>>>>     switch (env->spr[SPR_BOOKE_MAS0]&   MAS0_WQ_MASK) {
>>>>>>>>>     case MAS0_WQ_ALWAYS:
>>>>>>>>> @@ -4289,8 +4291,12 @@ void helper_booke206_tlbwe(void)
>>>>>>>>>         tlb->mas1 |= (tlbncfg&   TLBnCFG_MINSIZE)>>   12;
>>>>>>>>>     }
>>>>>>>>>
>>>>>>>>> -    /* XXX needs to change when supporting 64-bit e500 */
>>>>>>>>> -    tlb->mas2 = env->spr[SPR_BOOKE_MAS2]&   0xffffffff;
>>>>>>>>> +    /* Make a mask from TLB size to discard invalid bits in EPN field */
>>>>>>>>> +    mask = ~(booke206_tlb_to_page_size(env, tlb)
>>>>>>>> This breaks execution of -cpu with qemu-system-ppc64, no?
>>>>>>> -cpu e500 I mean of course :).
>>>>>>>
>>>>>> Maybe but I don't see why...
>>>>> Because the effective address might be padded to be negative, rendering lots of f's in the upper 32 bits.
>>>> Sorry I don't understand, can you provide an example?
>>> Sure, just try to run your guest kernel with qemu-system-ppc64 and it will break :).
>>>
>>> lis r1, 0x8000
>>> lwz r2, 0(r1)
>>>
>>> With qemu-system-ppc, this will translate to a read at 0x80000000. For qemu-system-ppc64 however, r1 contains 0xffffffff80000000, no?
>>>
>> r1 contains 0xffffffff80000000 but the effective address for lwz is
>> 0x0000000080000000. See below.
>>
>>>>> Do you maybe have an idea how this works for 64-bit BookE hardware? How does it make sure that a TLB entry only covers the lower 32 bits of the EA when running 32-bit user space?
>>>>>
>>>> No I don't know 64-bit BookE hardware. But I don't see why this would be
>>>> a special case. A 32-bit address would be padded with zeros to get a
>>>> 64-bit address to compare with EPN.
>>>>
>>>> 0x00100000 ->   0x0000000000100000
>>>>
>>>> It's up to the OS to provide a good mapping in a 32-bit process (i.e.
>>>> use 32-bit EPN).
>>> Hrm. This is not about the OS, it's about the TLB. Does TLB matching restrict itself to the low 32 bits when not in a 64-bit context? If so, we could add that check and make it work again :).
>>>
>>>
>> OK I have the answer from PowerISA RM:
>>
>> 5.7.1 32-Bit Mode
>>
>> The computation of the 64-bit effective address is independent of
>> whether the thread is in 32-bit mode or 64-bit mode. In 32-bit mode
>> (MSRSF=0), the high-order 32 bits of the 64-bit effective address are
>> treated as zeros for the purpose of addressing storage. This applies to
>> both data accesses and instruction fetches. It applies independent of
>> whether address translation is enabled or disabled. This truncation of
>> the effective address is the only respect in which storage accesses in
>> 32-bit mode differ from those in 64-bit mode.
> 
> This is Book3S, no? Maybe it applies to BookE too with s/SF/CM/?
> 

Sorry I didn't check, but you're right it's from Book3 S.

>> 6.11.4.8 32-bit and 64-bit Specific MMU Behavior
>>
>> MMU behavior is largely unaffected by whether the thread is in 32-bit
>> computation mode (MSRCM=0) or 64-bit computation mode (MSRCM=1). The
>> only differences occur in the EPN field of the TLB entry and the EPN
>> field of MAS2. The differences are summarized here.
>>
>>   - Executing a tlbwe instruction in 32-bit mode will set bits 0:31 of
>>     the TLB EPN field to zero unless MAS0ATSEL is set, in which case
>>     those bits are not written to zero.
> 
> Aha! That's the one. We should mask the EA as 32-bit when !MSR_CM. Could you either add that to your patch or add it to another patch? :)
> 
> The above logic is the reason we have the &= 0xffffffff in the code right now, which your patch removes.
> 

Will do.

-- 
Fabien Chouteau