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

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

[Fabien Chouteau]

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) - 1);
+    /* Add a mask for page attributes */
+    mask |= MAS2_ACM | MAS2_VLE | MAS2_W | MAS2_I | MAS2_M | MAS2_G | MAS2_E;
+
+    tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & mask;
 
     if (!(tlbncfg & TLBnCFG_IPROT)) {
         /* no IPROT supported by TLB */
-- 
1.7.9.5

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

[Fabien Chouteau]

Any comment?

On 05/09/2012 03:28 PM, Fabien Chouteau 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) - 1);
> +    /* Add a mask for page attributes */
> +    mask |= MAS2_ACM | MAS2_VLE | MAS2_W | MAS2_I | MAS2_M | MAS2_G | MAS2_E;
> +
> +    tlb->mas2 = env->spr[SPR_BOOKE_MAS2] & mask;
>  
>      if (!(tlbncfg & TLBnCFG_IPROT)) {
>          /* no IPROT supported by TLB */


-- 
Fabien Chouteau

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

[Alexander Graf]

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?

(sorry for the late reply)


Alex

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

[Alexander Graf]

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

Alex

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

[Fabien Chouteau]

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

-- 
Fabien Chouteau

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

[Alexander Graf]

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


Alex

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

[Fabien Chouteau]

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?

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

-- 
Fabien Chouteau

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

[Alexander Graf]

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?

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


Alex

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

[Fabien Chouteau]

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.

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.

-- 
Fabien Chouteau

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

[Alexander Graf]

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

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

[Fabien Chouteau]

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