Hi Dmitry,
On Sat, Jun 15, 2019 at 12:35:39AM +0000, Dmitry Korotin wrote:
> Added missing EHB (Execution Hazard Barrier) in mtc0 -> mfc0 sequence.
> Mips documentation Volume III (rev 6.03) table 8.1.
It would be good to describe the problem you saw here - ie. mention that
without this execution hazard barrier it's possible for the value read
back from the KScratch register to be the value from before the mtc0.
Also probably good to mention which CPUs the problem has been seen on.
Information like this can be really useful when making decisions about
stable backports, or for others who come across the patch later & just
want to figure out why you wrote it.
> Signed-off-by: Dmitry Korotin <dkorotin@xxxxxxxxxxxx>
> ---
> arch/mips/mm/tlbex.c | 32 ++++++++++++++++++++++----------
> 1 files changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/arch/mips/mm/tlbex.c b/arch/mips/mm/tlbex.c
> index 65b6e85..bf7f131 100644
> --- a/arch/mips/mm/tlbex.c
> +++ b/arch/mips/mm/tlbex.c
> @@ -391,6 +391,7 @@ static struct work_registers build_get_work_registers(u32 **p)
> static void build_restore_work_registers(u32 **p)
> {
> if (scratch_reg >= 0) {
> + uasm_i_ehb(p);
> UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg);
> return;
> }
> @@ -668,10 +669,12 @@ static void build_restore_pagemask(u32 **p, struct uasm_reloc **r,
> uasm_i_mtc0(p, 0, C0_PAGEMASK);
> uasm_il_b(p, r, lid);
> }
> - if (scratch_reg >= 0)
> + if (scratch_reg >= 0) {
> + uasm_i_ehb(p);
> UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg);
> - else
> + } else {
> UASM_i_LW(p, 1, scratchpad_offset(0), 0);
> + }
> } else {
> /* Reset default page size */
> if (PM_DEFAULT_MASK >> 16) {
> @@ -938,10 +941,12 @@ void build_get_pmde64(u32 **p, struct uasm_label **l, struct uasm_reloc **r,
> uasm_i_jr(p, ptr);
>
> if (mode == refill_scratch) {
> - if (scratch_reg >= 0)
> + if (scratch_reg >= 0) {
> + uasm_i_ehb(p);
> UASM_i_MFC0(p, 1, c0_kscratch(), scratch_reg);
> - else
> + } else {
> UASM_i_LW(p, 1, scratchpad_offset(0), 0);
> + }
> } else {
> uasm_i_nop(p);
> }
> @@ -1258,6 +1263,7 @@ struct mips_huge_tlb_info {
> UASM_i_MTC0(p, odd, C0_ENTRYLO1); /* load it */
>
> if (c0_scratch_reg >= 0) {
> + uasm_i_ehb(p);
> UASM_i_MFC0(p, scratch, c0_kscratch(), c0_scratch_reg);
> build_tlb_write_entry(p, l, r, tlb_random);
> uasm_l_leave(l, *p);
> @@ -1603,15 +1609,19 @@ static void build_setup_pgd(void)
> uasm_i_dinsm(&p, a0, 0, 29, 64 - 29);
> uasm_l_tlbl_goaround1(&l, p);
> UASM_i_SLL(&p, a0, a0, 11);
> - uasm_i_jr(&p, 31);
> UASM_i_MTC0(&p, a0, C0_CONTEXT);
> + uasm_i_ehb(&p);
> + uasm_i_jr(&p, 31);
> + uasm_i_nop(&p);
Could the ehb go in the JR's delay slot here?
> } else {
> /* PGD in c0_KScratch */
> - uasm_i_jr(&p, 31);
> if (cpu_has_ldpte)
> UASM_i_MTC0(&p, a0, C0_PWBASE);
> else
> UASM_i_MTC0(&p, a0, c0_kscratch(), pgd_reg);
> + uasm_i_ehb(&p);
> + uasm_i_jr(&p, 31);
> + uasm_i_nop(&p);
Likewise here.
> }
> #else
> #ifdef CONFIG_SMP
> @@ -1625,13 +1635,15 @@ static void build_setup_pgd(void)
> UASM_i_LA_mostly(&p, a2, pgdc);
> UASM_i_SW(&p, a0, uasm_rel_lo(pgdc), a2);
> #endif /* SMP */
> - uasm_i_jr(&p, 31);
>
> /* if pgd_reg is allocated, save PGD also to scratch register */
> - if (pgd_reg != -1)
> + if (pgd_reg != -1) {
> UASM_i_MTC0(&p, a0, c0_kscratch(), pgd_reg);
> - else
> - uasm_i_nop(&p);
> + uasm_i_ehb(&p);
> + }
> +
> + uasm_i_jr(&p, 31);
> + uasm_i_nop(&p);
And here too.
Thanks,
Paul
