[PATCH V15 1/6] target/mips: Fix PageMask with variable page size

Huacai Chen posted 6 patches 5 years, 3 months ago
Maintainers: Aurelien Jarno <aurelien@aurel32.net>, Aleksandar Rikalo <aleksandar.rikalo@syrmia.com>, "Philippe Mathieu-Daudé" <f4bug@amsat.org>, Jiaxun Yang <jiaxun.yang@flygoat.com>
There is a newer version of this series
[PATCH V15 1/6] target/mips: Fix PageMask with variable page size
Posted by Huacai Chen 5 years, 3 months ago
From: Jiaxun Yang <jiaxun.yang@flygoat.com>

Our current code assumed the target page size is always 4k
when handling PageMask and VPN2, however, variable page size
was just added to mips target and that's no longer true.

Fixes: ee3863b9d414 ("target/mips: Support variable page size")
Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
Signed-off-by: Huacai Chen <chenhc@lemote.com>
---
 target/mips/cp0_helper.c | 36 +++++++++++++++++++++++++++++-------
 target/mips/cpu.h        |  1 +
 2 files changed, 30 insertions(+), 7 deletions(-)

diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index 12143ac..d90ddd9 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -892,13 +892,35 @@ void helper_mtc0_memorymapid(CPUMIPSState *env, target_ulong arg1)
 
 void update_pagemask(CPUMIPSState *env, target_ulong arg1, int32_t *pagemask)
 {
-    uint64_t mask = arg1 >> (TARGET_PAGE_BITS + 1);
-    if (!(env->insn_flags & ISA_MIPS32R6) || (arg1 == ~0) ||
-        (mask == 0x0000 || mask == 0x0003 || mask == 0x000F ||
-         mask == 0x003F || mask == 0x00FF || mask == 0x03FF ||
-         mask == 0x0FFF || mask == 0x3FFF || mask == 0xFFFF)) {
-        env->CP0_PageMask = arg1 & (0x1FFFFFFF & (TARGET_PAGE_MASK << 1));
+    unsigned long mask;
+    int maskbits;
+
+    if (env->insn_flags & ISA_MIPS32R6) {
+        return;
+    }
+    /* Don't care MASKX as we don't support 1KB page */
+    mask = extract32((uint32_t)arg1, CP0PM_MASK, 16);
+    maskbits = find_first_zero_bit(&mask, 32);
+
+    /* Ensure no more set bit after first zero */
+    if (mask >> maskbits) {
+        goto invalid;
+    }
+    /* We don't support VTLB entry smaller than target page */
+    if ((maskbits + 12) < TARGET_PAGE_BITS) {
+        goto invalid;
     }
+    env->CP0_PageMask = mask << CP0PM_MASK;
+
+    return;
+
+invalid:
+    /*
+     * When invalid, ensure the value is bigger than or equal to
+     * the minimal but smaller than or equal to the maxium.
+     */
+    maskbits = MIN(16, MAX(maskbits, TARGET_PAGE_BITS - 12));
+    env->CP0_PageMask = ((1 << (16 + 1)) - 1) << CP0PM_MASK;
 }
 
 void helper_mtc0_pagemask(CPUMIPSState *env, target_ulong arg1)
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index d41579d..23f8c6f 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -619,6 +619,7 @@ struct CPUMIPSState {
  * CP0 Register 5
  */
     int32_t CP0_PageMask;
+#define CP0PM_MASK 13
     int32_t CP0_PageGrain_rw_bitmask;
     int32_t CP0_PageGrain;
 #define CP0PG_RIE 31
-- 
2.7.0


Re: [PATCH V15 1/6] target/mips: Fix PageMask with variable page size
Posted by Richard Henderson 5 years, 3 months ago
On 10/27/20 9:17 PM, Huacai Chen wrote:
> +invalid:
> +    /*
> +     * When invalid, ensure the value is bigger than or equal to
> +     * the minimal but smaller than or equal to the maxium.
> +     */
> +    maskbits = MIN(16, MAX(maskbits, TARGET_PAGE_BITS - 12));
> +    env->CP0_PageMask = ((1 << (16 + 1)) - 1) << CP0PM_MASK;

maskbits is unused.  Did you mean to use it?


r~

Re: [PATCH V15 1/6] target/mips: Fix PageMask with variable page size
Posted by chen huacai 5 years, 3 months ago
Hi, Richard,

On Wed, Oct 28, 2020 at 4:48 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 10/27/20 9:17 PM, Huacai Chen wrote:
> > +invalid:
> > +    /*
> > +     * When invalid, ensure the value is bigger than or equal to
> > +     * the minimal but smaller than or equal to the maxium.
> > +     */
> > +    maskbits = MIN(16, MAX(maskbits, TARGET_PAGE_BITS - 12));
> > +    env->CP0_PageMask = ((1 << (16 + 1)) - 1) << CP0PM_MASK;
>
> maskbits is unused.  Did you mean to use it?
This is redundant, will be removed.

Huacai
>
>
> r~



-- 
Huacai Chen