diff mbox

[2/7] target-mips: support Page Frame Number Extension field

Message ID 1430224874-18513-3-git-send-email-leon.alrae@imgtec.com
State New
Headers show

Commit Message

Leon Alrae April 28, 2015, 12:41 p.m. UTC 
Update tlb->PFN to contain PFN concatenated with PFNX. PFNX is 0 if large
physical address is not supported.

Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
---
 target-mips/op_helper.c | 32 ++++++++++++++++++++++++++------
 1 file changed, 26 insertions(+), 6 deletions(-)

Comments

James Hogan April 28, 2015, 1:35 p.m. UTC | #1 
On 28/04/15 13:41, Leon Alrae wrote:
> Update tlb->PFN to contain PFN concatenated with PFNX. PFNX is 0 if large
> physical address is not supported.
> 
> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> ---
>  target-mips/op_helper.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
> 
> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> index c9a60bd..6bff927 100644
> --- a/target-mips/op_helper.c
> +++ b/target-mips/op_helper.c
> @@ -1825,6 +1825,16 @@ static void r4k_mips_tlb_flush_extra (CPUMIPSState *env, int first)
>      }
>  }
>  
> +static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
> +{
> +#if defined(TARGET_MIPS64)
> +    return extract64(entrylo, 6, 54);
> +#else
> +    return extract64(entrylo, 6, 24) | /* PFN */
> +           (extract64(entrylo, 32, 32) << 24); /* PFNX */

Where does the 32,32 come from? The PRA I have seems to imply that PFNX
starts at bit 30 and goes up to bit 54.

That would of course also mean that the code for mfc0 EntryLo* needs
tweaking so that PFNX doesn't cause XI/RI bits to be set at bits 30,31
(haven't looked at other patches yet).

> +#endif
> +}
> +
>  static void r4k_fill_tlb(CPUMIPSState *env, int idx)
>  {
>      r4k_tlb_t *tlb;
> @@ -1848,13 +1858,13 @@ static void r4k_fill_tlb(CPUMIPSState *env, int idx)
>      tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
>      tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
>      tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
> -    tlb->PFN[0] = (env->CP0_EntryLo0 >> 6) << 12;
> +    tlb->PFN[0] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) << 12;
>      tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
>      tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
>      tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
>      tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
>      tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
> -    tlb->PFN[1] = (env->CP0_EntryLo1 >> 6) << 12;
> +    tlb->PFN[1] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) << 12;
>  }
>  
>  void r4k_helper_tlbinv(CPUMIPSState *env)
> @@ -1971,6 +1981,16 @@ void r4k_helper_tlbp(CPUMIPSState *env)
>      }
>  }
>  
> +static inline uint64_t get_entrylo_pfn_from_tlb(uint64_t tlb_pfn)
> +{
> +#if defined(TARGET_MIPS64)
> +    return tlb_pfn << 6;
> +#else
> +    return (extract64(tlb_pfn, 0, 24) << 6) | /* PFN */
> +           (extract64(tlb_pfn, 24, 32) << 32); /* PFNX */

same again. shouldn't it be 25 bits starting at bit 24, shifted to start
at bit 30?

Cheers
James

> +#endif
> +}
> +
>  void r4k_helper_tlbr(CPUMIPSState *env)
>  {
>      r4k_tlb_t *tlb;
> @@ -1997,12 +2017,12 @@ void r4k_helper_tlbr(CPUMIPSState *env)
>          env->CP0_PageMask = tlb->PageMask;
>          env->CP0_EntryLo0 = tlb->G | (tlb->V0 << 1) | (tlb->D0 << 2) |
>                          ((uint64_t)tlb->RI0 << CP0EnLo_RI) |
> -                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) |
> -                        (tlb->C0 << 3) | (tlb->PFN[0] >> 6);
> +                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) | (tlb->C0 << 3) |
> +                        get_entrylo_pfn_from_tlb(tlb->PFN[0] >> 12);
>          env->CP0_EntryLo1 = tlb->G | (tlb->V1 << 1) | (tlb->D1 << 2) |
>                          ((uint64_t)tlb->RI1 << CP0EnLo_RI) |
> -                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) |
> -                        (tlb->C1 << 3) | (tlb->PFN[1] >> 6);
> +                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) | (tlb->C1 << 3) |
> +                        get_entrylo_pfn_from_tlb(tlb->PFN[1] >> 12);
>      }
>  }
>  
>
James Hogan April 28, 2015, 1:47 p.m. UTC | #2 
On 28/04/15 14:35, James Hogan wrote:
> 
> 
> On 28/04/15 13:41, Leon Alrae wrote:
>> Update tlb->PFN to contain PFN concatenated with PFNX. PFNX is 0 if large
>> physical address is not supported.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
>>  target-mips/op_helper.c | 32 ++++++++++++++++++++++++++------
>>  1 file changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
>> index c9a60bd..6bff927 100644
>> --- a/target-mips/op_helper.c
>> +++ b/target-mips/op_helper.c
>> @@ -1825,6 +1825,16 @@ static void r4k_mips_tlb_flush_extra (CPUMIPSState *env, int first)
>>      }
>>  }
>>  
>> +static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
>> +{
>> +#if defined(TARGET_MIPS64)
>> +    return extract64(entrylo, 6, 54);
>> +#else
>> +    return extract64(entrylo, 6, 24) | /* PFN */
>> +           (extract64(entrylo, 32, 32) << 24); /* PFNX */
> 
> Where does the 32,32 come from? The PRA I have seems to imply that PFNX
> starts at bit 30 and goes up to bit 54.

Ah right, I guess this is from the output of MFHC0/MTHC0 instructions.
The specification of those instructions specify the mangling they do:

GPR[rt]_31:0 <- data_61..30
GPR[rt]_63..32 <- (data_61)^31 // sign-extend

Note how the field it extracts starts at bit 30. IMO the stored values
of EntryLo PFN should be continuous rather than split up like the
MFHC0/MTHC0 versions.

Cheers
James

> 
> That would of course also mean that the code for mfc0 EntryLo* needs
> tweaking so that PFNX doesn't cause XI/RI bits to be set at bits 30,31
> (haven't looked at other patches yet).
> 
>> +#endif
>> +}
>> +
>>  static void r4k_fill_tlb(CPUMIPSState *env, int idx)
>>  {
>>      r4k_tlb_t *tlb;
>> @@ -1848,13 +1858,13 @@ static void r4k_fill_tlb(CPUMIPSState *env, int idx)
>>      tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
>>      tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
>>      tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
>> -    tlb->PFN[0] = (env->CP0_EntryLo0 >> 6) << 12;
>> +    tlb->PFN[0] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) << 12;
>>      tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
>>      tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
>>      tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
>>      tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
>>      tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
>> -    tlb->PFN[1] = (env->CP0_EntryLo1 >> 6) << 12;
>> +    tlb->PFN[1] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) << 12;
>>  }
>>  
>>  void r4k_helper_tlbinv(CPUMIPSState *env)
>> @@ -1971,6 +1981,16 @@ void r4k_helper_tlbp(CPUMIPSState *env)
>>      }
>>  }
>>  
>> +static inline uint64_t get_entrylo_pfn_from_tlb(uint64_t tlb_pfn)
>> +{
>> +#if defined(TARGET_MIPS64)
>> +    return tlb_pfn << 6;
>> +#else
>> +    return (extract64(tlb_pfn, 0, 24) << 6) | /* PFN */
>> +           (extract64(tlb_pfn, 24, 32) << 32); /* PFNX */
> 
> same again. shouldn't it be 25 bits starting at bit 24, shifted to start
> at bit 30?
> 
> Cheers
> James
> 
>> +#endif
>> +}
>> +
>>  void r4k_helper_tlbr(CPUMIPSState *env)
>>  {
>>      r4k_tlb_t *tlb;
>> @@ -1997,12 +2017,12 @@ void r4k_helper_tlbr(CPUMIPSState *env)
>>          env->CP0_PageMask = tlb->PageMask;
>>          env->CP0_EntryLo0 = tlb->G | (tlb->V0 << 1) | (tlb->D0 << 2) |
>>                          ((uint64_t)tlb->RI0 << CP0EnLo_RI) |
>> -                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) |
>> -                        (tlb->C0 << 3) | (tlb->PFN[0] >> 6);
>> +                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) | (tlb->C0 << 3) |
>> +                        get_entrylo_pfn_from_tlb(tlb->PFN[0] >> 12);
>>          env->CP0_EntryLo1 = tlb->G | (tlb->V1 << 1) | (tlb->D1 << 2) |
>>                          ((uint64_t)tlb->RI1 << CP0EnLo_RI) |
>> -                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) |
>> -                        (tlb->C1 << 3) | (tlb->PFN[1] >> 6);
>> +                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) | (tlb->C1 << 3) |
>> +                        get_entrylo_pfn_from_tlb(tlb->PFN[1] >> 12);
>>      }
>>  }
>>  
>>
>
Leon Alrae April 28, 2015, 3:59 p.m. UTC | #3 
Hi James,

On 28/04/2015 14:35, James Hogan wrote:
> 
> 
> On 28/04/15 13:41, Leon Alrae wrote:
>> Update tlb->PFN to contain PFN concatenated with PFNX. PFNX is 0 if large
>> physical address is not supported.
>>
>> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
>> ---
>>  target-mips/op_helper.c | 32 ++++++++++++++++++++++++++------
>>  1 file changed, 26 insertions(+), 6 deletions(-)
>>
>> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
>> index c9a60bd..6bff927 100644
>> --- a/target-mips/op_helper.c
>> +++ b/target-mips/op_helper.c
>> @@ -1825,6 +1825,16 @@ static void r4k_mips_tlb_flush_extra (CPUMIPSState *env, int first)
>>      }
>>  }
>>  
>> +static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
>> +{
>> +#if defined(TARGET_MIPS64)
>> +    return extract64(entrylo, 6, 54);
>> +#else
>> +    return extract64(entrylo, 6, 24) | /* PFN */
>> +           (extract64(entrylo, 32, 32) << 24); /* PFNX */
> 
> Where does the 32,32 come from? The PRA I have seems to imply that PFNX
> starts at bit 30 and goes up to bit 54.

This comes directly from MIPS32 PRA (I presume you are looking at MIPS64
PRA). Note that EntryLo.PFNX starts at bit 32 as there is 2-bit gap
occupied by RI/XI (unlike MIPS64 where it starts at bit 30).

Regards,
Leon
James Hogan April 28, 2015, 9:39 p.m. UTC | #4 
On Tue, Apr 28, 2015 at 04:59:20PM +0100, Leon Alrae wrote:
> Hi James,
> 
> On 28/04/2015 14:35, James Hogan wrote:
> > 
> > 
> > On 28/04/15 13:41, Leon Alrae wrote:
> >> Update tlb->PFN to contain PFN concatenated with PFNX. PFNX is 0 if large
> >> physical address is not supported.
> >>
> >> Signed-off-by: Leon Alrae <leon.alrae@imgtec.com>
> >> ---
> >>  target-mips/op_helper.c | 32 ++++++++++++++++++++++++++------
> >>  1 file changed, 26 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
> >> index c9a60bd..6bff927 100644
> >> --- a/target-mips/op_helper.c
> >> +++ b/target-mips/op_helper.c
> >> @@ -1825,6 +1825,16 @@ static void r4k_mips_tlb_flush_extra (CPUMIPSState *env, int first)
> >>      }
> >>  }
> >>  
> >> +static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
> >> +{
> >> +#if defined(TARGET_MIPS64)
> >> +    return extract64(entrylo, 6, 54);
> >> +#else
> >> +    return extract64(entrylo, 6, 24) | /* PFN */
> >> +           (extract64(entrylo, 32, 32) << 24); /* PFNX */
> > 
> > Where does the 32,32 come from? The PRA I have seems to imply that PFNX
> > starts at bit 30 and goes up to bit 54.
> 
> This comes directly from MIPS32 PRA (I presume you are looking at MIPS64
> PRA).

Right, the MIPS32 64-bit representation is the same as what mfc0 and
mfhc0 would give you, and I see that the raw EntryLo representation in
QEMU already has XI/RI in different places depending on MIPS64/MIPS32,
i.e. following how each PRA represents the raw state differently.

I think I wasn't expecting that because 32-bit kernels can run on MIPS64
hardware using the same mfc0/mfhc0 instructions, so having a single
internal representation in QEMU seemed simpler & less fragile, since the
same source code needs to support both MIPS32 and MIPS64 anyway.

> Note that EntryLo.PFNX starts at bit 32 as there is 2-bit gap
> occupied by RI/XI (unlike MIPS64 where it starts at bit 30).

Well, from a programmer point of view mfc0 and mfhc0 with EntryLo behave
identically on both MIPS32 and MIPS64.

Thanks
James
Leon Alrae April 29, 2015, 3:31 p.m. UTC | #5 
> I think I wasn't expecting that because 32-bit kernels can run on MIPS64
> hardware using the same mfc0/mfhc0 instructions, so having a single
> internal representation in QEMU seemed simpler & less fragile, since the
> same source code needs to support both MIPS32 and MIPS64 anyway.

In my opinion implementing MIPS32 details in a different way than
specified in MIPS32 PRA would be actually more fragile. That could cause
just more confusion when implementing future MIPS32 PRA changes
affecting this area.

Leon
diff mbox

Patch

diff --git a/target-mips/op_helper.c b/target-mips/op_helper.c
index c9a60bd..6bff927 100644
--- a/target-mips/op_helper.c
+++ b/target-mips/op_helper.c
@@ -1825,6 +1825,16 @@  static void r4k_mips_tlb_flush_extra (CPUMIPSState *env, int first)
     }
 }
 
+static inline uint64_t get_tlb_pfn_from_entrylo(uint64_t entrylo)
+{
+#if defined(TARGET_MIPS64)
+    return extract64(entrylo, 6, 54);
+#else
+    return extract64(entrylo, 6, 24) | /* PFN */
+           (extract64(entrylo, 32, 32) << 24); /* PFNX */
+#endif
+}
+
 static void r4k_fill_tlb(CPUMIPSState *env, int idx)
 {
     r4k_tlb_t *tlb;
@@ -1848,13 +1858,13 @@  static void r4k_fill_tlb(CPUMIPSState *env, int idx)
     tlb->C0 = (env->CP0_EntryLo0 >> 3) & 0x7;
     tlb->XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
     tlb->RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
-    tlb->PFN[0] = (env->CP0_EntryLo0 >> 6) << 12;
+    tlb->PFN[0] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo0) << 12;
     tlb->V1 = (env->CP0_EntryLo1 & 2) != 0;
     tlb->D1 = (env->CP0_EntryLo1 & 4) != 0;
     tlb->C1 = (env->CP0_EntryLo1 >> 3) & 0x7;
     tlb->XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
     tlb->RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
-    tlb->PFN[1] = (env->CP0_EntryLo1 >> 6) << 12;
+    tlb->PFN[1] = get_tlb_pfn_from_entrylo(env->CP0_EntryLo1) << 12;
 }
 
 void r4k_helper_tlbinv(CPUMIPSState *env)
@@ -1971,6 +1981,16 @@  void r4k_helper_tlbp(CPUMIPSState *env)
     }
 }
 
+static inline uint64_t get_entrylo_pfn_from_tlb(uint64_t tlb_pfn)
+{
+#if defined(TARGET_MIPS64)
+    return tlb_pfn << 6;
+#else
+    return (extract64(tlb_pfn, 0, 24) << 6) | /* PFN */
+           (extract64(tlb_pfn, 24, 32) << 32); /* PFNX */
+#endif
+}
+
 void r4k_helper_tlbr(CPUMIPSState *env)
 {
     r4k_tlb_t *tlb;
@@ -1997,12 +2017,12 @@  void r4k_helper_tlbr(CPUMIPSState *env)
         env->CP0_PageMask = tlb->PageMask;
         env->CP0_EntryLo0 = tlb->G | (tlb->V0 << 1) | (tlb->D0 << 2) |
                         ((uint64_t)tlb->RI0 << CP0EnLo_RI) |
-                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) |
-                        (tlb->C0 << 3) | (tlb->PFN[0] >> 6);
+                        ((uint64_t)tlb->XI0 << CP0EnLo_XI) | (tlb->C0 << 3) |
+                        get_entrylo_pfn_from_tlb(tlb->PFN[0] >> 12);
         env->CP0_EntryLo1 = tlb->G | (tlb->V1 << 1) | (tlb->D1 << 2) |
                         ((uint64_t)tlb->RI1 << CP0EnLo_RI) |
-                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) |
-                        (tlb->C1 << 3) | (tlb->PFN[1] >> 6);
+                        ((uint64_t)tlb->XI1 << CP0EnLo_XI) | (tlb->C1 << 3) |
+                        get_entrylo_pfn_from_tlb(tlb->PFN[1] >> 12);
     }
 }