[PATCH 0/3] MIPS system emulation miscellaneous fixes

[PATCH 0/3] MIPS system emulation miscellaneous fixes

[Jiaxun Yang]

Hi all,

I was trying to build a MIPS VirtIO board[1] for QEMU that is able
to work with all processors we support.

When I was bring up varoius CPUs on that board I noticed some issues
with the system emulation code that I'm fixing in this series.

Thanks.

- Jiaxun
[1]: https://gitlab.com/FlyGoat/qemu/-/tree/mips-virt

Jiaxun Yang (3):
  target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F
  target/mips: Cast offset field of Octeon BBIT to int16_t
  target/mips: Disable DSP ASE for Octeon68XX

 target/mips/cpu-defs.c.inc         | 4 ++--
 target/mips/cpu.c                  | 6 ++++++
 target/mips/tcg/octeon_translate.c | 2 +-
 3 files changed, 9 insertions(+), 3 deletions(-)

-- 
2.37.1 (Apple Git-137.1)

[PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Jiaxun Yang]

As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
should is not writeable and hardcoded to 1.

Without those bits set, kernel is unable to access XKPHYS address
segmant. So just set them up on CPU reset.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 target/mips/cpu.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/mips/cpu.c b/target/mips/cpu.c
index d0a76b95f7..a870901bfa 100644
--- a/target/mips/cpu.c
+++ b/target/mips/cpu.c
@@ -304,6 +304,12 @@ static void mips_cpu_reset(DeviceState *dev)
     env->CP0_EntryHi_ASID_mask = (env->CP0_Config5 & (1 << CP0C5_MI)) ?
             0x0 : (env->CP0_Config4 & (1 << CP0C4_AE)) ? 0x3ff : 0xff;
     env->CP0_Status = (1 << CP0St_BEV) | (1 << CP0St_ERL);
+    if (env->insn_flags & INSN_LOONGSON2F) {
+        /* Loongson-2F has those bits hardcoded to 1 */
+        env->CP0_Status |= (1 << CP0St_KX) | (1 << CP0St_SX) |
+                            (1 << CP0St_UX);
+    }
+
     /*
      * Vectored interrupts not implemented, timer on int 7,
      * no performance counters.
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Philippe Mathieu-Daudé]

On 29/10/22 04:00, Jiaxun Yang wrote:
> As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
> should is not writeable and hardcoded to 1.
> 
> Without those bits set, kernel is unable to access XKPHYS address
> segmant. So just set them up on CPU reset.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   target/mips/cpu.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
> index d0a76b95f7..a870901bfa 100644
> --- a/target/mips/cpu.c
> +++ b/target/mips/cpu.c
> @@ -304,6 +304,12 @@ static void mips_cpu_reset(DeviceState *dev)
>       env->CP0_EntryHi_ASID_mask = (env->CP0_Config5 & (1 << CP0C5_MI)) ?
>               0x0 : (env->CP0_Config4 & (1 << CP0C4_AE)) ? 0x3ff : 0xff;
>       env->CP0_Status = (1 << CP0St_BEV) | (1 << CP0St_ERL);
> +    if (env->insn_flags & INSN_LOONGSON2F) {
> +        /* Loongson-2F has those bits hardcoded to 1 */
> +        env->CP0_Status |= (1 << CP0St_KX) | (1 << CP0St_SX) |
> +                            (1 << CP0St_UX);
> +    }

Don't we want to update CP0_Status_rw_bitmask in Loongson-2F
entry in mips_defs[] instead?

Re: [PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Jiaxun Yang]

> 2022年10月29日 18:44，Philippe Mathieu-Daudé <philmd@linaro.org> 写道：
> 
> On 29/10/22 04:00, Jiaxun Yang wrote:
>> As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
>> should is not writeable and hardcoded to 1.
>> Without those bits set, kernel is unable to access XKPHYS address
>> segmant. So just set them up on CPU reset.
>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>> ---
>>  target/mips/cpu.c | 6 ++++++
>>  1 file changed, 6 insertions(+)
>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>> index d0a76b95f7..a870901bfa 100644
>> --- a/target/mips/cpu.c
>> +++ b/target/mips/cpu.c
>> @@ -304,6 +304,12 @@ static void mips_cpu_reset(DeviceState *dev)
>>      env->CP0_EntryHi_ASID_mask = (env->CP0_Config5 & (1 << CP0C5_MI)) ?
>>              0x0 : (env->CP0_Config4 & (1 << CP0C4_AE)) ? 0x3ff : 0xff;
>>      env->CP0_Status = (1 << CP0St_BEV) | (1 << CP0St_ERL);
>> +    if (env->insn_flags & INSN_LOONGSON2F) {
>> +        /* Loongson-2F has those bits hardcoded to 1 */
>> +        env->CP0_Status |= (1 << CP0St_KX) | (1 << CP0St_SX) |
>> +                            (1 << CP0St_UX);
>> +    }
> 
> Don't we want to update CP0_Status_rw_bitmask in Loongson-2F
> entry in mips_defs[] instead?

Write to those bits is already disabled by CP0_Status_rw_bitmask. However real hardware
had those bits set to 1 but QEMU default them to 0…

Enable writing to those bits can also make kernel work but it mismatches actual hardware
behavior.

Thanks.
---
Jiaxun Yang

Re: [PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Philippe Mathieu-Daudé]

On 29/10/22 21:50, Jiaxun Yang wrote:
> 
> 
>> 2022年10月29日 18:44，Philippe Mathieu-Daudé <philmd@linaro.org> 写道：
>>
>> On 29/10/22 04:00, Jiaxun Yang wrote:
>>> As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
>>> should is not writeable and hardcoded to 1.
>>> Without those bits set, kernel is unable to access XKPHYS address
>>> segmant. So just set them up on CPU reset.
>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> ---
>>>   target/mips/cpu.c | 6 ++++++
>>>   1 file changed, 6 insertions(+)
>>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>>> index d0a76b95f7..a870901bfa 100644
>>> --- a/target/mips/cpu.c
>>> +++ b/target/mips/cpu.c
>>> @@ -304,6 +304,12 @@ static void mips_cpu_reset(DeviceState *dev)
>>>       env->CP0_EntryHi_ASID_mask = (env->CP0_Config5 & (1 << CP0C5_MI)) ?
>>>               0x0 : (env->CP0_Config4 & (1 << CP0C4_AE)) ? 0x3ff : 0xff;
>>>       env->CP0_Status = (1 << CP0St_BEV) | (1 << CP0St_ERL);
>>> +    if (env->insn_flags & INSN_LOONGSON2F) {
>>> +        /* Loongson-2F has those bits hardcoded to 1 */
>>> +        env->CP0_Status |= (1 << CP0St_KX) | (1 << CP0St_SX) |
>>> +                            (1 << CP0St_UX);
>>> +    }
>>
>> Don't we want to update CP0_Status_rw_bitmask in Loongson-2F
>> entry in mips_defs[] instead?
> 
> Write to those bits is already disabled by CP0_Status_rw_bitmask. However real hardware
> had those bits set to 1 but QEMU default them to 0…
> 
> Enable writing to those bits can also make kernel work but it mismatches actual hardware
> behavior.

On "龙芯 2F 处理器用户手册 (0.1 版, 2007 年 8 月\0\0)"
Section 5.10 Status 寄存器(12) (page 57),
CP0_Status bits 5..7 are 0.

Can you share your "Loongson-2F processor user manual" doc?

Re: [PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Jiaxun Yang]

> 2022年10月30日 00:19，Philippe Mathieu-Daudé <philmd@linaro.org> 写道：
> 
> On 29/10/22 21:50, Jiaxun Yang wrote:
>>> 2022年10月29日 18:44，Philippe Mathieu-Daudé <philmd@linaro.org> 写道：
>>> 
>>> On 29/10/22 04:00, Jiaxun Yang wrote:
>>>> As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
>>>> should is not writeable and hardcoded to 1.
>>>> Without those bits set, kernel is unable to access XKPHYS address
>>>> segmant. So just set them up on CPU reset.
>>>> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>>> ---
>>>>  target/mips/cpu.c | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>> diff --git a/target/mips/cpu.c b/target/mips/cpu.c
>>>> index d0a76b95f7..a870901bfa 100644
>>>> --- a/target/mips/cpu.c
>>>> +++ b/target/mips/cpu.c
>>>> @@ -304,6 +304,12 @@ static void mips_cpu_reset(DeviceState *dev)
>>>>      env->CP0_EntryHi_ASID_mask = (env->CP0_Config5 & (1 << CP0C5_MI)) ?
>>>>              0x0 : (env->CP0_Config4 & (1 << CP0C4_AE)) ? 0x3ff : 0xff;
>>>>      env->CP0_Status = (1 << CP0St_BEV) | (1 << CP0St_ERL);
>>>> +    if (env->insn_flags & INSN_LOONGSON2F) {
>>>> +        /* Loongson-2F has those bits hardcoded to 1 */
>>>> +        env->CP0_Status |= (1 << CP0St_KX) | (1 << CP0St_SX) |
>>>> +                            (1 << CP0St_UX);
>>>> +    }
>>> 
>>> Don't we want to update CP0_Status_rw_bitmask in Loongson-2F
>>> entry in mips_defs[] instead?
>> Write to those bits is already disabled by CP0_Status_rw_bitmask. However real hardware
>> had those bits set to 1 but QEMU default them to 0…
>> Enable writing to those bits can also make kernel work but it mismatches actual hardware
>> behavior.
> 
> On "龙芯 2F 处理器用户手册 (0.1 版, 2007 年 8 月)"
> Section 5.10 Status 寄存器(12) (page 57),
> CP0_Status bits 5..7 are 0.
> 
> Can you share your "Loongson-2F processor user manual" doc?

Ah sorry the document was marked as “company confidential” so I’m not sure if I can share
the whole doc. It was updated in 2016 with version 1.8. The latest document I can find in wild
Is version 1.5 [1] but it didn’t cover newer chip reversions.

There is a footnote saying value of those bits was changed in later chips, to translate it says:

"Since LS2F04 those bits was refined to 1. As in LS2F we had implemented a single 64 bit addressing
model and it is mostly compatible with MIPS64 64 bit addressing model.”

It is obvious that without KX and UX bit 64 bit kernel won’t work but my Lemote Fuloong box is running
64 bit kernel along with n64 AOSC/Retro [2] user-space rootfs. For SX bit as LS2F supports XSSEG it
should work as well, though nobody take MIPS supervisor mode serious :-)

[1]: https://github.com/loongson-community/docs/tree/master/2F
[2]: https://wiki.aosc.io/aosc-os/retro/intro/
---
Jiaxun Yang

Re: [PATCH 1/3] target/mips: Set CP0St_{KX, SX, UX} for Loongson-2F

[Richard Henderson]

On 10/29/22 13:00, Jiaxun Yang wrote:
> As per "Loongson-2F processor user manual", CP0St_{KX, SX, UX}
> should is not writeable and hardcoded to 1.
> 
> Without those bits set, kernel is unable to access XKPHYS address
> segmant. So just set them up on CPU reset.
> 
> Signed-off-by: Jiaxun Yang<jiaxun.yang@flygoat.com>
> ---
>   target/mips/cpu.c | 6 ++++++
>   1 file changed, 6 insertions(+)

Based on down-thread discussion of the manual:
Acked-by: Richard Henderson <richard.henderson@linaro.org>


r~

[PATCH 2/3] target/mips: Cast offset field of Octeon BBIT to int16_t

[Jiaxun Yang]

As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference
Manual" offset field is signed 16 bit value. However arg_BBIT.offset
is unsigned. We need to cast it as signed to do address calculation.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 target/mips/tcg/octeon_translate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/mips/tcg/octeon_translate.c b/target/mips/tcg/octeon_translate.c
index 6a207d2e7e..e8f2277c51 100644
--- a/target/mips/tcg/octeon_translate.c
+++ b/target/mips/tcg/octeon_translate.c
@@ -38,7 +38,7 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
     }
 
     ctx->hflags |= MIPS_HFLAG_BC;
-    ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
+    ctx->btarget = ctx->base.pc_next + 4 + (int16_t)a->offset * 4;
     ctx->hflags |= MIPS_HFLAG_BDS32;
 
     tcg_temp_free(t0);
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH 2/3] target/mips: Cast offset field of Octeon BBIT to int16_t

[Philippe Mathieu-Daudé]

On 29/10/22 04:00, Jiaxun Yang wrote:
> As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference
> Manual" offset field is signed 16 bit value. However arg_BBIT.offset
> is unsigned. We need to cast it as signed to do address calculation.
> 
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
>   target/mips/tcg/octeon_translate.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/mips/tcg/octeon_translate.c b/target/mips/tcg/octeon_translate.c
> index 6a207d2e7e..e8f2277c51 100644
> --- a/target/mips/tcg/octeon_translate.c
> +++ b/target/mips/tcg/octeon_translate.c
> @@ -38,7 +38,7 @@ static bool trans_BBIT(DisasContext *ctx, arg_BBIT *a)
>       }
>   
>       ctx->hflags |= MIPS_HFLAG_BC;
> -    ctx->btarget = ctx->base.pc_next + 4 + a->offset * 4;
> +    ctx->btarget = ctx->base.pc_next + 4 + (int16_t)a->offset * 4;
>       ctx->hflags |= MIPS_HFLAG_BDS32;
>   
>       tcg_temp_free(t0);

In target/mips/tcg/octeon.decode:

-BBIT         11 set:1 . 10 rs:5 ..... offset:16 p=%bbit_p
+BBIT         11 set:1 . 10 rs:5 ..... offset:s16 p=%bbit_p

[PATCH 3/3] target/mips: Disable DSP ASE for Octeon68XX

[Jiaxun Yang]

I don't have access to Octeon68XX hardware but accroading to
my investigation Octeon never had DSP ASE support.

As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference
Manual" CP0C3_DSPP is reserved bit and read as 0. Also I do have
access to a Ubiquiti Edgerouter 4 which has Octeon CN7130 processor
and I can confirm CP0C3_DSPP is read as 0 on that processor.

Further more, in linux kernel:
arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
cpu_has_dsp is overridden as 0.

So I believe we shouldn't emulate DSP in QEMU as well.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 target/mips/cpu-defs.c.inc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/mips/cpu-defs.c.inc b/target/mips/cpu-defs.c.inc
index 7f53c94ec8..480e60aeec 100644
--- a/target/mips/cpu-defs.c.inc
+++ b/target/mips/cpu-defs.c.inc
@@ -934,7 +934,7 @@ const mips_def_t mips_defs[] =
                        (1 << CP0C1_DS) | (4 << CP0C1_DL) | (1 << CP0C1_DA) |
                        (1 << CP0C1_PC) | (1 << CP0C1_WR) | (1 << CP0C1_EP),
         .CP0_Config2 = MIPS_CONFIG2,
-        .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA) | (1 << CP0C3_DSPP) ,
+        .CP0_Config3 = MIPS_CONFIG3 | (1 << CP0C3_LPA),
         .CP0_Config4 = MIPS_CONFIG4 | (1U << CP0C4_M) |
                        (0x3c << CP0C4_KScrExist) | (1U << CP0C4_MMUExtDef) |
                        (3U << CP0C4_MMUSizeExt),
@@ -946,7 +946,7 @@ const mips_def_t mips_defs[] =
         .CP0_Status_rw_bitmask = 0x12F8FFFF,
         .SEGBITS = 42,
         .PABITS = 49,
-        .insn_flags = CPU_MIPS64R2 | INSN_OCTEON | ASE_DSP,
+        .insn_flags = CPU_MIPS64R2 | INSN_OCTEON,
         .mmu_type = MMU_TYPE_R4000,
     },
 
-- 
2.37.1 (Apple Git-137.1)

Re: [PATCH 3/3] target/mips: Disable DSP ASE for Octeon68XX

[Richard Henderson]

On 10/29/22 13:00, Jiaxun Yang wrote:
> I don't have access to Octeon68XX hardware but accroading to
> my investigation Octeon never had DSP ASE support.
> 
> As per "Cavium Networks OCTEON Plus CN50XX Hardware Reference
> Manual" CP0C3_DSPP is reserved bit and read as 0. Also I do have
> access to a Ubiquiti Edgerouter 4 which has Octeon CN7130 processor
> and I can confirm CP0C3_DSPP is read as 0 on that processor.
> 
> Further more, in linux kernel:
> arch/mips/include/asm/mach-cavium-octeon/cpu-feature-overrides.h
> cpu_has_dsp is overridden as 0.
> 
> So I believe we shouldn't emulate DSP in QEMU as well.
> 
> Signed-off-by: Jiaxun Yang<jiaxun.yang@flygoat.com>
> ---
>   target/mips/cpu-defs.c.inc | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Richard Henderson <richard.henderson@linaro.org>

r~