Patchwork PPC: booke206: Check for min/max TLB entry size

login
register
mail settings
Submitter Alexander Graf
Date Jan. 20, 2012, 1:21 p.m.
Message ID <1327065698-7538-1-git-send-email-agraf@suse.de>
Download mbox | patch
Permalink /patch/137029/
State New
Headers show

Comments

Alexander Graf - Jan. 20, 2012, 1:21 p.m.
When setting a TLB entry, we need to check if the TLB we're putting it in
actually supports the given size. According to the 2.06 PowerPC ISA, a
value that's out of range results in the minimum page size for the TLB
to be used.

Signed-off-by: Alexander Graf <agraf@suse.de>

---

v1 -> v2:

  - fix min/max check
Scott Wood - Jan. 20, 2012, 8:01 p.m.
On 01/20/2012 07:21 AM, Alexander Graf wrote:
> When setting a TLB entry, we need to check if the TLB we're putting it in
> actually supports the given size. According to the 2.06 PowerPC ISA, a
> value that's out of range results in the minimum page size for the TLB
> to be used.

The ISA says, "If the page size specified by
MAS1TSIZE is not supported by the specified array, the
tlbwe may be performed as if TSIZE were some imple-
mentation-dependent value, or an Illegal Instruction
exception occurs."

In practice, what this means on e500 is that TLB0 (which only supports
one page size) ignores TSIZE.  I'm not sure what happens when you write
an entry to TLB1 with an invalid TSIZE.

> +    /* XXX only applies for MAV 1.0 */
> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
> +        /* set to min size */
> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
> +    }

You could just implement a bitmap now, which will work for MAV 2.0 as well.

Especially since we're using the MAV 2.0 definition of tsize already, so
min/max isn't an accurate way to describe what we support.

-Scott
Alexander Graf - Jan. 21, 2012, 2:43 a.m.
Am 20.01.2012 um 21:01 schrieb Scott Wood <scottwood@freescale.com>:

> On 01/20/2012 07:21 AM, Alexander Graf wrote:
>> When setting a TLB entry, we need to check if the TLB we're putting it in
>> actually supports the given size. According to the 2.06 PowerPC ISA, a
>> value that's out of range results in the minimum page size for the TLB
>> to be used.
> 
> The ISA says, "If the page size specified by
> MAS1TSIZE is not supported by the specified array, the
> tlbwe may be performed as if TSIZE were some imple-
> mentation-dependent value, or an Illegal Instruction
> exception occurs."

Ah, I looked at MMUCSR0 behavior (page 955 in 2.06B) where it stated that invalid sizes mean MINSIZE. Apparently it's not defined for tlbwe though.

However, if we declare the implementation dependent value to be MINSIZE for us, we're good. If you have any idea what hardware does, that would be great. Otherwise when I get the chance I could of course. Also give it a try myself :).

> 
> In practice, what this means on e500 is that TLB0 (which only supports
> one page size) ignores TSIZE.  

Yes, that part is actually written out

> I'm not sure what happens when you write
> an entry to TLB1 with an invalid TSIZE.

What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.

However for now I think wde 're ok by not modeling every odd corner case.

> 
>> +    /* XXX only applies for MAV 1.0 */
>> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
>> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
>> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
>> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
>> +        /* set to min size */
>> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
>> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
>> +    }
> 
> You could just implement a bitmap now, which will work for MAV 2.0 as well.
> 
> Especially since we're using the MAV 2.0 definition of tsize already, so
> min/max isn't an accurate way to describe what we support.

Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)

Would you like to have a function called that returns a bitmap of supported sizes for the TLB depending on its MAV value based on either TLBnCFG or TLBnPS? We could then check if that size value bit is set.

Alex

> 
> -Scott
>
Scott Wood - Jan. 23, 2012, 5:28 p.m.
On 01/20/2012 08:43 PM, Alexander Graf wrote:
> 
> 
> Am 20.01.2012 um 21:01 schrieb Scott Wood <scottwood@freescale.com>:
>> I'm not sure what happens when you write
>> an entry to TLB1 with an invalid TSIZE.
> 
> What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.

AFAIK it's not documented what e500mc does for invalid sizes in TLB1, so
I think anything that complies with the architecture's statement of any
supported size is OK.

> However for now I think wde 're ok by not modeling every odd corner case.

Sure.  I was just curious about the architectural statement.

>>> +    /* XXX only applies for MAV 1.0 */
>>> +    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
>>> +    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
>>> +    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
>>> +    if ((size_tlb > size_max) || (size_tlb < size_min)) {
>>> +        /* set to min size */
>>> +        tlb->mas1 &= ~MAS1_TSIZE_MASK;
>>> +        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
>>> +    }
>>
>> You could just implement a bitmap now, which will work for MAV 2.0 as well.
>>
>> Especially since we're using the MAV 2.0 definition of tsize already, so
>> min/max isn't an accurate way to describe what we support.
> 
> Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)
> 
> Would you like to have a function called that returns a bitmap of
> supported sizes for the TLB depending on its MAV value based on
> either TLBnCFG or TLBnPS? We could then check if that size value bit
> is set.

Yes, use a bitmap internally regardless of how the programming model
says we convey the information to the target code.

-Scott
Alexander Graf - Jan. 23, 2012, 5:30 p.m.
On 01/23/2012 06:28 PM, Scott Wood wrote:
> On 01/20/2012 08:43 PM, Alexander Graf wrote:
>>
>> Am 20.01.2012 um 21:01 schrieb Scott Wood<scottwood@freescale.com>:
>>> I'm not sure what happens when you write
>>> an entry to TLB1 with an invalid TSIZE.
>> What it says, the ISA means it's implementation dependent. What e500mc actually implements is an different question. Which still needs to be answered.
> AFAIK it's not documented what e500mc does for invalid sizes in TLB1, so
> I think anything that complies with the architecture's statement of any
> supported size is OK.
>
>> However for now I think wde 're ok by not modeling every odd corner case.
> Sure.  I was just curious about the architectural statement.
>
>>>> +    /* XXX only applies for MAV 1.0 */
>>>> +    size_tlb = (tlb->mas1&  MAS1_TSIZE_MASK)>>  (MAS1_TSIZE_SHIFT + 1);
>>>> +    size_min = (tlbncfg&  TLBnCFG_MINSIZE)>>  TLBnCFG_MINSIZE_SHIFT;
>>>> +    size_max = (tlbncfg&  TLBnCFG_MAXSIZE)>>  TLBnCFG_MAXSIZE_SHIFT;
>>>> +    if ((size_tlb>  size_max) || (size_tlb<  size_min)) {
>>>> +        /* set to min size */
>>>> +        tlb->mas1&= ~MAS1_TSIZE_MASK;
>>>> +        tlb->mas1 |= size_min<<  (MAS1_TSIZE_SHIFT + 1);
>>>> +    }
>>> You could just implement a bitmap now, which will work for MAV 2.0 as well.
>>>
>>> Especially since we're using the MAV 2.0 definition of tsize already, so
>>> min/max isn't an accurate way to describe what we support.
>> Not sure I follow. In MAV 1.0 the size constraints are defined in TLBnCFG, while for MAV 2.0 they are defned in their own bitmap registers (TLBnPS)
>>
>> Would you like to have a function called that returns a bitmap of
>> supported sizes for the TLB depending on its MAV value based on
>> either TLBnCFG or TLBnPS? We could then check if that size value bit
>> is set.
> Yes, use a bitmap internally regardless of how the programming model
> says we convey the information to the target code.

Already done :).


Alex

Patch

diff --git a/target-ppc/op_helper.c b/target-ppc/op_helper.c
index 6339c95..8cd0224 100644
--- a/target-ppc/op_helper.c
+++ b/target-ppc/op_helper.c
@@ -4228,6 +4228,7 @@  void helper_booke206_tlbwe(void)
 {
     uint32_t tlbncfg, tlbn;
     ppcmas_tlb_t *tlb;
+    uint32_t size_tlb, size_min, size_max;
 
     switch (env->spr[SPR_BOOKE_MAS0] & MAS0_WQ_MASK) {
     case MAS0_WQ_ALWAYS:
@@ -4273,6 +4274,16 @@  void helper_booke206_tlbwe(void)
         tlb->mas1 &= ~MAS1_IPROT;
     }
 
+    /* XXX only applies for MAV 1.0 */
+    size_tlb = (tlb->mas1 & MAS1_TSIZE_MASK) >> (MAS1_TSIZE_SHIFT + 1);
+    size_min = (tlbncfg & TLBnCFG_MINSIZE) >> TLBnCFG_MINSIZE_SHIFT;
+    size_max = (tlbncfg & TLBnCFG_MAXSIZE) >> TLBnCFG_MAXSIZE_SHIFT;
+    if ((size_tlb > size_max) || (size_tlb < size_min)) {
+        /* set to min size */
+        tlb->mas1 &= ~MAS1_TSIZE_MASK;
+        tlb->mas1 |= size_min << (MAS1_TSIZE_SHIFT + 1);
+    }
+
     if (booke206_tlb_to_page_size(env, tlb) == TARGET_PAGE_SIZE) {
         tlb_flush_page(env, tlb->mas2 & MAS2_EPN_MASK);
     } else {