Re: [PATCH] MIPS: Limit check_bugs32() under CONFIG_32BIT

[Jiaxun Yang]

ä 2020å4æ11æ GMT+08:00 äå3:37:24, YunQiang Su <wzssyqa@xxxxxxxxx> åå:
>Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> ä2020å4æ9æåå äå11:47åéï
>>
>>
>>
>> ä 2020å4æ9æ GMT+08:00 äå11:07:46, "Maciej W. Rozycki"
><macro@xxxxxxxxxxxxxx> åå:
>> >On Thu, 9 Apr 2020, Jiaxun Yang wrote:
>> >
>> >> > > There is no need to build and call check_bugs32() under
>> >> > > CONFIG_64BIT, just limit it under CONFIG_32BIT.
>> >> >
>> >> > Since 32bit is subset of 64bit, and due to the code, I think
>that
>> >the
>> >> > initial purpose
>> >> > of check_bugs32() is also willing to run even with CONFIG_64BIT.
>> >> >
>> >> > For example, if we have a CPU which is 64bit, and work well on
>> >64bit
>> >> > mode, while has a bug only on 32bit mode, check_bugs32 should be
>> >used
>> >> > here.
>> >> >
>> >> > Loongson's 3A 1000 is the example, I cannot support FP32 mode
>well.
>> >>
>> >> In this case bugs32 only contains a workaround for MIPS34K, which
>is
>> >a
>> >> MIPS32 processor. It's safe to do so.
>> >
>> > This is because commit c65a5480ff29 ("[MIPS] Fix potential latency
>> >problem due to non-atomic cpu_wait.") moved the other generic
>> >workaround
>> >elsewhere.
>> >
>> > The intent has been since historical commit 450ad16ba0ab ("Get rid
>of
>> >arch/mips64/kernel.  9116 lines of code gone.") that `check_bugs32'
>is
>> >for
>> >generic errata affecting both 32-bit and 64-bit operation (e.g.
>32-bit
>> >instructions, which naturally may occur in both cases) and
>> >`check_bugs64'
>> >is for errata affecting 64-bit operation only (e.g. 64-bit
>> >instructions).
>> >
>> >But currently it appears we have no generic errata handled, as
>surely a
>> >
>> >34K erratum cannot affect 64-bit operation.  So I think such a
>change
>> >makes sense in principle (if a generic erratum appears in the future
>we
>> >
>> >can add a third category, which includes workarounds that are always
>> >applied), but I think it has to be made in a cleaner way.
>> >
>> >Specifically `check_errata' has to be renamed to `check_errata32',
>some
>> >
>> >commentary added as to the intent, and last but not least a proper
>> >change
>> >description added that not only repeats what the change does (and
>what
>> >everyone sees regardless), but actually justifies why the change is
>> >made.
>> >Saying: "There is no need[...]" does not tell us *why* there is no
>> >need.
>> >
>> >> But my suggestion is if you're going to clean-up bugs and
>workarounds
>> >> you'd better establish a file for silicon bugs and provide Kconfig
>> >> options to enable & disable them. Manage bug dependencies by
>Kconfig
>> >> will be easier.
>> >
>> > Why is using Kconfig supposed to be better?  Several configurations
>> >support multiple processor types (e.g. swappable CPU daugthercards
>or
>> >FPGA
>> >soft-cores) and having to list CPU types across platforms as CPUs
>are
>> >added is going to be a maintenance nightmare.  Whereas having
>> >workarounds
>> >or panics associated with run-time determination of the actual CPU
>type
>> >
>> >guarantees they will trigger where necessary.  The use of `init'
>> >sections
>> >assures the reclaim of memory for use after bootstrap.
>>
>> Actually I meant let bug checks depends on Kconfig's CPU selection.
>>
>
>I don't think this is a good idea. Since the routine is executed single
>time,
>so it is not performance critical.
>And Kconfig for per-cpu is bad for the future if we want to try to
>archive
>single kernel image.

You can only select one kind of CPU one time,
according to our current design.
e.g. MIPS32R2 (Indicating MIPS32R2 IP Cores from MIPS company),
Ingenic Xburst, Loongson64, R4000.

We won't create a new entry for a kind of CPU unless
the difference is significant.
But when that happens, filtering out unnecessary code on certain
platforms can be beneficial.

Thanks

-- 
Jiaxun Yang