diff mbox series

[V14,1/8] target/mips: Fix PageMask with variable page size

Message ID 1602831120-3377-2-git-send-email-chenhc@lemote.com
State New
Headers show
Series mips: Add Loongson-3 machine support | expand

Commit Message

chen huacai Oct. 16, 2020, 6:51 a.m. UTC 
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(-)

Comments

Philippe Mathieu-Daudé Oct. 16, 2020, 3:15 p.m. UTC | #1 
On 10/16/20 8:51 AM, Huacai Chen wrote:
> 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 de64add038..f3478d826b 100644
> --- a/target/mips/cp0_helper.c
> +++ b/target/mips/cp0_helper.c
> @@ -867,13 +867,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)
> @@ -1104,7 +1126,7 @@ void helper_mthc0_saar(CPUMIPSState *env, target_ulong arg1)
>   void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
>   {
>       target_ulong old, val, mask;
> -    mask = (TARGET_PAGE_MASK << 1) | env->CP0_EntryHi_ASID_mask;
> +    mask = ~((1 << 14) - 1) | env->CP0_EntryHi_ASID_mask;
>       if (((env->CP0_Config4 >> CP0C4_IE) & 0x3) >= 2) {
>           mask |= 1 << CP0EnHi_EHINV;
>       }
> diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> index 7cf7f5239f..9c8bb23807 100644
> --- a/target/mips/cpu.h
> +++ b/target/mips/cpu.h
> @@ -618,6 +618,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
> 

Malta test failing:

[    0.000000] Linux version 4.5.0-2-4kc-malta 
(debian-kernel@lists.debian.org) (gcc version 5.3.1 20160519 (Debian 
5.3.1-20) ) #1 Debian 4.5.5-1 (2016-05-29)
[    0.000000] earlycon: Early serial console at I/O port 0x3f8 (options 
'38400n8')
[    0.000000] bootconsole [uart0] enabled
[    0.000000] CPU0 revision is: 00019300 (MIPS 24Kc)
[    0.000000] FPU revision is: 00739300
[    0.000000] MIPS: machine is mti,malta
[...]
Freeing unused kernel memory: 412K (80979000 - 809e0000)
do_page_fault(): sending SIGSEGV to mount for invalid write access to 
0018a000
epc = 77848a54 in libc-2.27.so[7782f000+177000]
ra  = 779d0618 in ld-2.27.so[779bf000+24000]
do_page_fault(): sending SIGSEGV to ln for invalid write access to 0018a000
epc = 778d4a54 in libc-2.27.so[778bb000+177000]
ra  = 77a5c618 in ld-2.27.so[77a4b000+24000]
do_page_fault(): sending SIGSEGV to S01logging for invalid write access 
to 0018a000
epc = 77d08a54 in libc-2.27.so[77cef000+177000]
ra  = 77e90618 in ld-2.27.so[77e7f000+24000]
do_page_fault(): sending SIGSEGV to S20urandom for invalid write access 
to 0018a000
epc = 76ee4a54 in libc-2.27.so[76ecb000+177000]
ra  = 7706c618 in ld-2.27.so[7705b000+24000]
do_page_fault(): sending SIGSEGV to ifup for invalid write access to 
0018a000
epc = 77974a54 in libc-2.27.so[7795b000+177000]
ra  = 77afc618 in ld-2.27.so[77aeb000+24000]
do_page_fault(): sending SIGSEGV to awk for invalid read access from 
00000000
epc = 00000000 in busybox[400000+d8000]
ra  = 77248110 in libc-2.27.so[770fb000+177000]
do_page_fault(): sending SIGSEGV to cat for invalid write access to 0018a000
epc = 77484a54 in libc-2.27.so[7746b000+177000]
ra  = 7760c618 in ld-2.27.so[775fb000+24000]
do_page_fault(): sending SIGSEGV to run.sh for invalid write access to 
0018a000
epc = 76e88a54 in libc-2.27.so[76e6f000+177000]
ra  = 77010618 in ld-2.27.so[76fff000+24000]
qemu-system-mips: terminating on signal 2

Please run the QEMU tests.

The easiest way is to push your series on GitLab.

Regards,

Phil.
Huacai Chen Oct. 20, 2020, 1:38 a.m. UTC | #2 
Hi, Philippe,

On Fri, Oct 16, 2020 at 11:15 PM Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 10/16/20 8:51 AM, Huacai Chen wrote:
> > 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 de64add038..f3478d826b 100644
> > --- a/target/mips/cp0_helper.c
> > +++ b/target/mips/cp0_helper.c
> > @@ -867,13 +867,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)
> > @@ -1104,7 +1126,7 @@ void helper_mthc0_saar(CPUMIPSState *env, target_ulong arg1)
> >   void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
> >   {
> >       target_ulong old, val, mask;
> > -    mask = (TARGET_PAGE_MASK << 1) | env->CP0_EntryHi_ASID_mask;
> > +    mask = ~((1 << 14) - 1) | env->CP0_EntryHi_ASID_mask;
> >       if (((env->CP0_Config4 >> CP0C4_IE) & 0x3) >= 2) {
> >           mask |= 1 << CP0EnHi_EHINV;
> >       }
> > diff --git a/target/mips/cpu.h b/target/mips/cpu.h
> > index 7cf7f5239f..9c8bb23807 100644
> > --- a/target/mips/cpu.h
> > +++ b/target/mips/cpu.h
> > @@ -618,6 +618,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
> >
>
> Malta test failing:
>
> [    0.000000] Linux version 4.5.0-2-4kc-malta
> (debian-kernel@lists.debian.org) (gcc version 5.3.1 20160519 (Debian
> 5.3.1-20) ) #1 Debian 4.5.5-1 (2016-05-29)
> [    0.000000] earlycon: Early serial console at I/O port 0x3f8 (options
> '38400n8')
> [    0.000000] bootconsole [uart0] enabled
> [    0.000000] CPU0 revision is: 00019300 (MIPS 24Kc)
> [    0.000000] FPU revision is: 00739300
> [    0.000000] MIPS: machine is mti,malta
> [...]
> Freeing unused kernel memory: 412K (80979000 - 809e0000)
> do_page_fault(): sending SIGSEGV to mount for invalid write access to
> 0018a000
> epc = 77848a54 in libc-2.27.so[7782f000+177000]
> ra  = 779d0618 in ld-2.27.so[779bf000+24000]
> do_page_fault(): sending SIGSEGV to ln for invalid write access to 0018a000
> epc = 778d4a54 in libc-2.27.so[778bb000+177000]
> ra  = 77a5c618 in ld-2.27.so[77a4b000+24000]
> do_page_fault(): sending SIGSEGV to S01logging for invalid write access
> to 0018a000
> epc = 77d08a54 in libc-2.27.so[77cef000+177000]
> ra  = 77e90618 in ld-2.27.so[77e7f000+24000]
> do_page_fault(): sending SIGSEGV to S20urandom for invalid write access
> to 0018a000
> epc = 76ee4a54 in libc-2.27.so[76ecb000+177000]
> ra  = 7706c618 in ld-2.27.so[7705b000+24000]
> do_page_fault(): sending SIGSEGV to ifup for invalid write access to
> 0018a000
> epc = 77974a54 in libc-2.27.so[7795b000+177000]
> ra  = 77afc618 in ld-2.27.so[77aeb000+24000]
> do_page_fault(): sending SIGSEGV to awk for invalid read access from
> 00000000
> epc = 00000000 in busybox[400000+d8000]
> ra  = 77248110 in libc-2.27.so[770fb000+177000]
> do_page_fault(): sending SIGSEGV to cat for invalid write access to 0018a000
> epc = 77484a54 in libc-2.27.so[7746b000+177000]
> ra  = 7760c618 in ld-2.27.so[775fb000+24000]
> do_page_fault(): sending SIGSEGV to run.sh for invalid write access to
> 0018a000
> epc = 76e88a54 in libc-2.27.so[76e6f000+177000]
> ra  = 77010618 in ld-2.27.so[76fff000+24000]
> qemu-system-mips: terminating on signal 2
>
> Please run the QEMU tests.
OK, we are investigating.

>
> The easiest way is to push your series on GitLab.
>
> Regards,
>
> Phil.
diff mbox series

Patch

diff --git a/target/mips/cp0_helper.c b/target/mips/cp0_helper.c
index de64add038..f3478d826b 100644
--- a/target/mips/cp0_helper.c
+++ b/target/mips/cp0_helper.c
@@ -867,13 +867,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)
@@ -1104,7 +1126,7 @@  void helper_mthc0_saar(CPUMIPSState *env, target_ulong arg1)
 void helper_mtc0_entryhi(CPUMIPSState *env, target_ulong arg1)
 {
     target_ulong old, val, mask;
-    mask = (TARGET_PAGE_MASK << 1) | env->CP0_EntryHi_ASID_mask;
+    mask = ~((1 << 14) - 1) | env->CP0_EntryHi_ASID_mask;
     if (((env->CP0_Config4 >> CP0C4_IE) & 0x3) >= 2) {
         mask |= 1 << CP0EnHi_EHINV;
     }
diff --git a/target/mips/cpu.h b/target/mips/cpu.h
index 7cf7f5239f..9c8bb23807 100644
--- a/target/mips/cpu.h
+++ b/target/mips/cpu.h
@@ -618,6 +618,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