On 7/3/20 12:53 AM, Hauke Mehrtens wrote:
> This resolves the hazard between the mtc0 in the change_c0_status() and
> the mfc0 in configure_exception_vector(). Without resolving this hazard
> configure_exception_vector() could read an old value and would restore
> this old value again. This would revert the changes change_c0_status()
> did. I checked this by printing out the read_c0_status() at the end of
> per_cpu_trap_init() and the ST0_MX is not set without this patch.
>
> The hazard is documented in the MIPS Architecture Reference Manual Vol.
> III: MIPS32/microMIPS32 Privileged Resource Architecture (MD00088), rev
> 6.03 table 8.1 which includes:
>
> Producer | Consumer | Hazard
> ----------|----------|----------------------------
> mtc0 | mfc0 | any coprocessor 0 register
>
> I saw this hazard on an Atheros AR9344 rev 2 SoC with a MIPS 74Kc CPU.
> There the change_c0_status() function would activate the DSPen by
> setting ST0_MX in the c0_status register. This was reverted and then the
> system got a DSP exception when the DSP registers were saved in
> save_dsp() in the first process switch. The crash looks like this:
>
> [ 0.089999] Mount-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
> [ 0.097796] Mountpoint-cache hash table entries: 1024 (order: 0, 4096 bytes, linear)
> [ 0.107070] Kernel panic - not syncing: Unexpected DSP exception
> [ 0.113470] Rebooting in 1 seconds..
>
> We saw this problem in OpenWrt only on the MIPS 74Kc based Atheros SoCs,
> not on the 24Kc based SoCs. We only saw it with kernel 5.4 not with
> kernel 4.19, in addition we had to use GCC 8.4 or 9.X, with GCC 8.3 it
> did not happen.
>
> In the kernel I bisected this problem to commit 9012d011660e ("compiler:
> allow all arches to enable CONFIG_OPTIMIZE_INLINING"), but when this was
> reverted it also happened after commit 172dcd935c34b ("MIPS: Always
> allocate exception vector for MIPSr2+").
>
> Commit 0b24cae4d535 ("MIPS: Add missing EHB in mtc0 -> mfc0 sequence.")
> does similar changes to a different file. I am not sure if there are
> more places affected by this problem.
>
> Signed-off-by: Hauke Mehrtens <hauke@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
> ---
> arch/mips/kernel/traps.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/mips/kernel/traps.c b/arch/mips/kernel/traps.c
> index 7c32c956156a..1234ea21dd8f 100644
> --- a/arch/mips/kernel/traps.c
> +++ b/arch/mips/kernel/traps.c
> @@ -2169,6 +2169,7 @@ static void configure_status(void)
>
> change_c0_status(ST0_CU|ST0_MX|ST0_RE|ST0_FR|ST0_BEV|ST0_TS|ST0_KX|ST0_SX|ST0_UX,
> status_set);
> + back_to_back_c0_hazard();
> }
>
> unsigned int hwrena;
>
The MIPS InterAptiv system software user manual suggests to put this
back_to_back_c0_hazard() close to the consumer. Should I move it there?
Should I also add a back_to_back_c0_hazard() close to the other places
which could be affected by similar problems even when I do not see them
at runtime?
Hauke
Attachment:
signature.asc
Description: OpenPGP digital signature
