[1/2] MIPS: Allow MIPS32R2 kernel to run on P5600 and M5150

  M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
backward compatible with MIPS32R2 there is no reason to forbid
M5150 and P5600 on MIPS32R2 kernel.

Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
---
 arch/mips/include/asm/cpu-type.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
  

On Mon, 29 May 2023, Jiaxun Yang wrote:

> M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
> backward compatible with MIPS32R2 there is no reason to forbid
> M5150 and P5600 on MIPS32R2 kernel.

 What problem are you trying to solve?  The CONFIG_SYS_HAS_CPU_* settings 
denote overall platform's support for the given CPU and have nothing to do 
with what architecture level a given kernel has been configured for.  You 
do need to get the settings right for your platform, just as you do in 
2/2, but this 1/2 part looks wrong to me.

 NB CPU_4KEC is double-listed as R1 and R2 because early revisions of the 
4KEc core were actually R1 before switching to R2, so this CPU can report 
either revision.

 I don't know why CPU_XBURST is also listed as both R1 and R2, the history 
looks convoluted with no explanation.  Paul, is the CPU also dual-revision 
or is it just a bug and it is supposed to be listed under one ISA revision
only, presumably R2?

  Maciej
  

Hi Maciej,

Le mardi 30 mai 2023 à 09:03 +0100, Maciej W. Rozycki a écrit :
> On Mon, 29 May 2023, Jiaxun Yang wrote:
> 
> > M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
> > backward compatible with MIPS32R2 there is no reason to forbid
> > M5150 and P5600 on MIPS32R2 kernel.
> 
>  What problem are you trying to solve?  The CONFIG_SYS_HAS_CPU_*
> settings 
> denote overall platform's support for the given CPU and have nothing
> to do 
> with what architecture level a given kernel has been configured for. 
> You 
> do need to get the settings right for your platform, just as you do
> in 
> 2/2, but this 1/2 part looks wrong to me.
> 
>  NB CPU_4KEC is double-listed as R1 and R2 because early revisions of
> the 
> 4KEc core were actually R1 before switching to R2, so this CPU can
> report 
> either revision.
> 
>  I don't know why CPU_XBURST is also listed as both R1 and R2, the
> history 
> looks convoluted with no explanation.  Paul, is the CPU also dual-
> revision 
> or is it just a bug and it is supposed to be listed under one ISA
> revision
> only, presumably R2?

The XBurst CPU is R1 in older Ingenic SoCs (JZ4760B and older), and R2
in newer SoCs (JZ4770 and newer).

Cheers,
-Paul
  

Hi Paul,

> >  I don't know why CPU_XBURST is also listed as both R1 and R2, the
> > history 
> > looks convoluted with no explanation.  Paul, is the CPU also dual-
> > revision 
> > or is it just a bug and it is supposed to be listed under one ISA
> > revision
> > only, presumably R2?
> 
> The XBurst CPU is R1 in older Ingenic SoCs (JZ4760B and older), and R2
> in newer SoCs (JZ4770 and newer).

 Great, thanks for confirming.  So the current arrangement is right and 
still we don't want to dual-list the P5600 or M5150.

  Maciej
  

> 2023年5月30日 09:03，Maciej W. Rozycki <macro@orcam.me.uk> 写道：
> 
> On Mon, 29 May 2023, Jiaxun Yang wrote:
> 
>> M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
>> backward compatible with MIPS32R2 there is no reason to forbid
>> M5150 and P5600 on MIPS32R2 kernel.
> 
> What problem are you trying to solve?  The CONFIG_SYS_HAS_CPU_* settings 
> denote overall platform's support for the given CPU and have nothing to do 
> with what architecture level a given kernel has been configured for.  You 
> do need to get the settings right for your platform, just as you do in 
> 2/2, but this 1/2 part looks wrong to me.

Well the universal target is to allow R2 generic kernel to run on R5 CPUs.
As R5 is backward compatible we can just have one universal kernel binary.

Allowing P5600 and M5150 to run on R2 kernel does not bring much overhead.
In fact only several bytes are added to kernel binary.

(Actually although M5150 is advertising as R5 it’s technically R2 because it does
not implement some features mandatory for R5.)

Thanks
- Jiaxun

> 
> NB CPU_4KEC is double-listed as R1 and R2 because early revisions of the 
> 4KEc core were actually R1 before switching to R2, so this CPU can report 
> either revision.
> 
> I don't know why CPU_XBURST is also listed as both R1 and R2, the history 
> looks convoluted with no explanation.  Paul, is the CPU also dual-revision 
> or is it just a bug and it is supposed to be listed under one ISA revision
> only, presumably R2?
> 
>  Maciej
  

On Tue, 30 May 2023, Jiaxun Yang wrote:

> >> M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
> >> backward compatible with MIPS32R2 there is no reason to forbid
> >> M5150 and P5600 on MIPS32R2 kernel.
> > 
> > What problem are you trying to solve?  The CONFIG_SYS_HAS_CPU_* settings 
> > denote overall platform's support for the given CPU and have nothing to do 
> > with what architecture level a given kernel has been configured for.  You 
> > do need to get the settings right for your platform, just as you do in 
> > 2/2, but this 1/2 part looks wrong to me.
> 
> Well the universal target is to allow R2 generic kernel to run on R5 CPUs.
> As R5 is backward compatible we can just have one universal kernel binary.

 Sure, but this change is not needed for it.  You just need to declare 
which ISA revisions your platform supports and leave `__get_cpu_type' 
alone.  It has worked like that for a decade now.

 Back in the day I used to run R1 kernels on R2 hardware myself.  And 
maybe MIPS IV on R1 even, as we had MIPS Malta CPU modules with both MIPS 
IV devices (QED RM5261/RM7061) and MIPS64r1 devices (MIPS 5Kc/20Kc/25Kf) 
and switching the kernel when swapping modules was a nuisance.  The Malta 
config still supports these devices although some may not exist anymore.

  Maciej
  

> 2023年5月30日 12:07，Maciej W. Rozycki <macro@orcam.me.uk> 写道：
> 
> On Tue, 30 May 2023, Jiaxun Yang wrote:
> 
>>>> M5150 and P5600 are two MIPS32R5 kernels, however as MIPS32R5 is
>>>> backward compatible with MIPS32R2 there is no reason to forbid
>>>> M5150 and P5600 on MIPS32R2 kernel.
>>> 
>>> What problem are you trying to solve?  The CONFIG_SYS_HAS_CPU_* settings 
>>> denote overall platform's support for the given CPU and have nothing to do 
>>> with what architecture level a given kernel has been configured for.  You 
>>> do need to get the settings right for your platform, just as you do in 
>>> 2/2, but this 1/2 part looks wrong to me.
>> 
>> Well the universal target is to allow R2 generic kernel to run on R5 CPUs.
>> As R5 is backward compatible we can just have one universal kernel binary.
> 
> Sure, but this change is not needed for it.  You just need to declare 
> which ISA revisions your platform supports and leave `__get_cpu_type' 
> alone.  It has worked like that for a decade now.

I’m afraid it won’t work as you expected.

Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
by compiler, because the codepath is marked as unreachable.

Thanks
- Jiaxun

> 
> Back in the day I used to run R1 kernels on R2 hardware myself.  And 
> maybe MIPS IV on R1 even, as we had MIPS Malta CPU modules with both MIPS 
> IV devices (QED RM5261/RM7061) and MIPS64r1 devices (MIPS 5Kc/20Kc/25Kf) 
> and switching the kernel when swapping modules was a nuisance.  The Malta 
> config still supports these devices although some may not exist anymore.
> 
>  Maciej
  

On Tue, 30 May 2023, Jiaxun Yang wrote:

> > Sure, but this change is not needed for it.  You just need to declare 
> > which ISA revisions your platform supports and leave `__get_cpu_type' 
> > alone.  It has worked like that for a decade now.
> 
> I’m afraid it won’t work as you expected.
> 
> Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
> by compiler, because the codepath is marked as unreachable.

 Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
reproduce it.

  Maciej
  

On Tue, May 30, 2023 at 01:16:32PM +0100, Maciej W. Rozycki wrote:
> On Tue, 30 May 2023, Jiaxun Yang wrote:
> 
> > > Sure, but this change is not needed for it.  You just need to declare 
> > > which ISA revisions your platform supports and leave `__get_cpu_type' 
> > > alone.  It has worked like that for a decade now.
> > 
> > I’m afraid it won’t work as you expected.
> > 
> > Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
> > by compiler, because the codepath is marked as unreachable.
> 
>  Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
> reproduce it.

I may have misunderstood something, but it seems like there is no such
problem on my P5600 system:

[fancer@mobilestation] kernel $ grep -r P5600 -B2 -A2 arch/mips/mm/c-r4k.c 
        case CPU_1004K:
        case CPU_INTERAPTIV:
        case CPU_P5600:
        case CPU_PROAPTIV:
        case CPU_M5150:
--
        case CPU_P6600:
        case CPU_M6250:
                pr_info("MIPS P5600 is here\n");
                if (!(read_c0_config7() & MIPS_CONF7_IAR) &&
                    (c->icache.waysize > PAGE_SIZE))

Log:
[    0.000000] Linux version 6.4.0-rc1-bt1-00235-gf9efd6b74b12-dirty (fancer@mobilestation) (mipsel-baikal-linux-gcc (GCC) 7.3.0, GNU ld (GNU Binutils) 2.30.0.20180208) #1
275 SMP PREEMPT Tue May 30 15:30:48 MSK 2023
[    0.000000] CPU0 revision is: 0001a830 (MIPS P5600)
[    0.000000] FPU revision is: 30f30320
[    0.000000] MSA revision is: 00000320
...
[    0.000000] VPE topology {1,1} total 2
[    0.000000] MIPS P5600 is here
...

-Serge(y)

> 
>   Maciej
  

On Tue, May 30, 2023 at 03:41:30PM +0300, Serge Semin wrote:
> On Tue, May 30, 2023 at 01:16:32PM +0100, Maciej W. Rozycki wrote:
> > On Tue, 30 May 2023, Jiaxun Yang wrote:
> > 
> > > > Sure, but this change is not needed for it.  You just need to declare 
> > > > which ISA revisions your platform supports and leave `__get_cpu_type' 
> > > > alone.  It has worked like that for a decade now.
> > > 
> > > I’m afraid it won’t work as you expected.
> > > 
> > > Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
> > > by compiler, because the codepath is marked as unreachable.
> > 
> >  Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
> > reproduce it.
> 
> I may have misunderstood something, but it seems like there is no such
> problem on my P5600 system:
> 
> [fancer@mobilestation] kernel $ grep -r P5600 -B2 -A2 arch/mips/mm/c-r4k.c 
>         case CPU_1004K:
>         case CPU_INTERAPTIV:
>         case CPU_P5600:
>         case CPU_PROAPTIV:
>         case CPU_M5150:
> --
>         case CPU_P6600:
>         case CPU_M6250:
>                 pr_info("MIPS P5600 is here\n");
>                 if (!(read_c0_config7() & MIPS_CONF7_IAR) &&
>                     (c->icache.waysize > PAGE_SIZE))
> 
> Log:
> [    0.000000] Linux version 6.4.0-rc1-bt1-00235-gf9efd6b74b12-dirty (fancer@mobilestation) (mipsel-baikal-linux-gcc (GCC) 7.3.0, GNU ld (GNU Binutils) 2.30.0.20180208) #1
> 275 SMP PREEMPT Tue May 30 15:30:48 MSK 2023
> [    0.000000] CPU0 revision is: 0001a830 (MIPS P5600)
> [    0.000000] FPU revision is: 30f30320
> [    0.000000] MSA revision is: 00000320
> ...
> [    0.000000] VPE topology {1,1} total 2
> [    0.000000] MIPS P5600 is here
> ...

Here is the CPU-related kernel configs:
root@msbt2:~# cat /proc/config.gz | gunzip | grep CPU_MIPS
# CONFIG_CPU_MIPS32_R2 is not set
# CONFIG_CPU_MIPS32_R5 is not set
# CONFIG_CPU_MIPS32_3_5_FEATURES is not set
CONFIG_CPU_MIPS32_R5_FEATURES=y
CONFIG_CPU_MIPS32_R5_XPA=y
CONFIG_SYS_HAS_CPU_MIPS32_R2=y
CONFIG_SYS_HAS_CPU_MIPS32_R3_5=y
CONFIG_SYS_HAS_CPU_MIPS32_R5=y
CONFIG_CPU_MIPS32=y
CONFIG_CPU_MIPSR5=y
CONFIG_CPU_MIPSR2_IRQ_VI=y
CONFIG_CPU_MIPSR2_IRQ_EI=y

-Serge(y)

> 
> -Serge(y)
> 
> > 
> >   Maciej
  

> 2023年5月30日 13:51，Serge Semin <fancer.lancer@gmail.com> 写道：
> 
> On Tue, May 30, 2023 at 03:41:30PM +0300, Serge Semin wrote:
>> On Tue, May 30, 2023 at 01:16:32PM +0100, Maciej W. Rozycki wrote:
>>> On Tue, 30 May 2023, Jiaxun Yang wrote:
>>> 
>>>>> Sure, but this change is not needed for it.  You just need to declare 
>>>>> which ISA revisions your platform supports and leave `__get_cpu_type' 
>>>>> alone.  It has worked like that for a decade now.
>>>> 
>>>> I’m afraid it won’t work as you expected.
>>>> 
>>>> Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
>>>> by compiler, because the codepath is marked as unreachable.
>>> 
>>> Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
>>> reproduce it.
>> 
>> I may have misunderstood something, but it seems like there is no such
>> problem on my P5600 system:
>> 
>> [fancer@mobilestation] kernel $ grep -r P5600 -B2 -A2 arch/mips/mm/c-r4k.c 
>>        case CPU_1004K:
>>        case CPU_INTERAPTIV:
>>        case CPU_P5600:
>>        case CPU_PROAPTIV:
>>        case CPU_M5150:
>> --
>>        case CPU_P6600:
>>        case CPU_M6250:
>>                pr_info("MIPS P5600 is here\n");
>>                if (!(read_c0_config7() & MIPS_CONF7_IAR) &&
>>                    (c->icache.waysize > PAGE_SIZE))
>> 
>> Log:
>> [    0.000000] Linux version 6.4.0-rc1-bt1-00235-gf9efd6b74b12-dirty (fancer@mobilestation) (mipsel-baikal-linux-gcc (GCC) 7.3.0, GNU ld (GNU Binutils) 2.30.0.20180208) #1
>> 275 SMP PREEMPT Tue May 30 15:30:48 MSK 2023
>> [    0.000000] CPU0 revision is: 0001a830 (MIPS P5600)
>> [    0.000000] FPU revision is: 30f30320
>> [    0.000000] MSA revision is: 00000320
>> ...
>> [    0.000000] VPE topology {1,1} total 2
>> [    0.000000] MIPS P5600 is here
>> ...
> 
> Here is the CPU-related kernel configs:
> root@msbt2:~# cat /proc/config.gz | gunzip | grep CPU_MIPS
> # CONFIG_CPU_MIPS32_R2 is not set
> # CONFIG_CPU_MIPS32_R5 is not set
> # CONFIG_CPU_MIPS32_3_5_FEATURES is not set
> CONFIG_CPU_MIPS32_R5_FEATURES=y
> CONFIG_CPU_MIPS32_R5_XPA=y
> CONFIG_SYS_HAS_CPU_MIPS32_R2=y
> CONFIG_SYS_HAS_CPU_MIPS32_R3_5=y
> CONFIG_SYS_HAS_CPU_MIPS32_R5=y
> CONFIG_CPU_MIPS32=y
> CONFIG_CPU_MIPSR5=y
> CONFIG_CPU_MIPSR2_IRQ_VI=y
> CONFIG_CPU_MIPSR2_IRQ_EI=y

I was trying to run kernel compiled with CONFIG_CPU_MIPS32_R2 on P5600.

Thanks
- Jiaxun

> 
> -Serge(y)
> 
>> 
>> -Serge(y)
>> 
>>> 
>>>  Maciej
  

> 2023年5月30日 13:16，Maciej W. Rozycki <macro@orcam.me.uk> 写道：
> 
> On Tue, 30 May 2023, Jiaxun Yang wrote:
> 
>>> Sure, but this change is not needed for it.  You just need to declare 
>>> which ISA revisions your platform supports and leave `__get_cpu_type' 
>>> alone.  It has worked like that for a decade now.
>> 
>> I’m afraid it won’t work as you expected.
>> 
>> Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
>> by compiler, because the codepath is marked as unreachable.
> 
> Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
> reproduce it.

Ok I see the problem, after applying patch 2 the issue is gone.
So actually only patch 2 is necessary.

The unreachable mark here leads gcc to generate some confusing code
and I misread it.

Sorry for the noise.

Thanks
- Jiaxun

> 
>  Maciej
  

On Tue, 30 May 2023, Jiaxun Yang wrote:

> > Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
> > reproduce it.
> 
> Ok I see the problem, after applying patch 2 the issue is gone.
> So actually only patch 2 is necessary.

 I'm glad we're on the same page then.

  Maciej
  

On Tue, May 30, 2023 at 02:10:04PM +0100, Jiaxun Yang wrote:
> 
> 
> > 2023年5月30日 13:16，Maciej W. Rozycki <macro@orcam.me.uk> 写道：
> > 
> > On Tue, 30 May 2023, Jiaxun Yang wrote:
> > 
> >>> Sure, but this change is not needed for it.  You just need to declare 
> >>> which ISA revisions your platform supports and leave `__get_cpu_type' 
> >>> alone.  It has worked like that for a decade now.
> >> 
> >> I’m afraid it won’t work as you expected.
> >> 
> >> Actually I ran into a problem that `case CPU_P5600` in c-r4k.c is optimised out
> >> by compiler, because the codepath is marked as unreachable.
> > 
> > Maybe there's a bug elsewhere then.  Send me your .config and I'll try to 
> > reproduce it.
> 

> Ok I see the problem, after applying patch 2 the issue is gone.
> So actually only patch 2 is necessary.
> 
> The unreachable mark here leads gcc to generate some confusing code
> and I misread it.
> 
> Sorry for the noise.

Great! Indeed enabling the SYS_HAS_CPU_MIPS32_R5 config is enough to
stop the CPU_P5600 being optimized out. Thanks for your work. Let me
know if you need some tests on another instance of the P5600 hw.

-Serge(y)

> 
> Thanks
> - Jiaxun
> 
> > 
> >  Maciej
>