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

[Jiaxun Yang]

ä 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.

It's guaranteed that you can only select one kind of CPU one time,
to prevent the overhead of checking bugs on irrelevant processors.

And we still have to check PRID/CPUTYPE during boot to enable
proper workarounds, because the Kconfig options are telling about the possibility,
which means a processor potentially has some kinds of bug.

In this case, M34K's errata should depends on or selected by 
CPU_MIPS32_R2 in Kconfig.

So there won't be any nightmare, but only reduced code :-)

Probably we can build-up a general framework for checks & workarounds,
and display affected bugs in /proc/cpuinfo?
That's my personal thought.

Thanks.

>
>OTOH I agree splitting off errata handling to a separate file may make 
>sense for structural reasons; we have it already for `check_bugs64'.
>
>  Maciej

-- 
Jiaxun Yang