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

[Alexander Graf <agraf@suse.de>]

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/?

> 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.


Alex