Re: [PATCH] riscv: Add ERRATA_MIPS_P8700_WFI to replace WFI with mips.pause

[Aleksa Paunovic <aleksa.paunovic@htecgroup.com>]

Hi Paul,

Thank you for taking the time to review the patch!


On 5/13/26 20:25, Paul Walmsley wrote:
> Hi,
>
> On Mon, 11 May 2026, Aleksa Paunovic via B4 Relay wrote:
>
>> From: Djordje Todorovic <djordje.todorovic@htecgroup.com>
>>
>> The MIPS P8700 has bugs with the WFI instruction.
> Has MIPS published an erratum for this?   If so, could you add a link to
> it in the patch description?  If not, could you please briefly describe
> the impact of the bug in the patch description, when WFI is used?

MIPS has not published an erratum for this. Briefly, a WFI instruction
after a mispredicted branch can result in erroneous address translation.
>> This errata uses the RISC-V alternatives framework to patch all WFI
>> instructions with the MIPS P8700 pause opcode (0x00501013) at runtime
>> when running on P8700 hardware.
>>
>> Two call sites are patched:
>>  - arch/riscv/kernel/head.S: secondary hart parking loop
>>  - arch/riscv/include/asm/processor.h: wait_for_interrupt()
>>
>> Signed-off-by: Djordje Todorovic <djordje.todorovic@htecgroup.com>
>> Signed-off-by: Aleksa Paunovic <aleksa.paunovic@htecgroup.com>
>> ---
>> This patch was tested on QEMU configured with eight P8700 harts,
>> as well as on the MIPS Boston board, configured with a single P8700 CPU.
>> Errata application was tested by disassembling with GDB on QEMU
>> and inserting an illegal instruction on the Boston board.
>> Correctness was tested with a combination of kselftests
>> and torture tests (rcu, locktorture), along with coremark testing.
> This kind of test report is very helpful!  Thank you.
>
>> ---
>>  arch/riscv/Kconfig.errata                    | 11 +++++++++++
>>  arch/riscv/errata/mips/errata.c              | 14 ++++++++++++++
>>  arch/riscv/include/asm/errata_list.h         | 27 +++++++++++++++++++++++++++
>>  arch/riscv/include/asm/errata_list_vendors.h |  5 +++--
>>  arch/riscv/include/asm/processor.h           |  3 ++-
>>  arch/riscv/kernel/head.S                     |  4 +++-
>>  6 files changed, 60 insertions(+), 4 deletions(-)
>>
> [ ... ]
>
>> diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
>> index 6694b5ccdcf85cfe7e767ea4de981b34f2b17b04..cbe90b19556203e1462cfb345b164c9061887e74 100644
>> --- a/arch/riscv/include/asm/errata_list.h
>> +++ b/arch/riscv/include/asm/errata_list.h
>> @@ -25,6 +25,16 @@ ALTERNATIVE(__stringify(RISCV_PTR do_page_fault),                  \
>>           __stringify(RISCV_PTR sifive_cip_453_page_fault_trp),       \
>>           SIFIVE_VENDOR_ID, ERRATA_SIFIVE_CIP_453,                    \
>>           CONFIG_ERRATA_SIFIVE_CIP_453)
>> +
>> +#ifdef CONFIG_ERRATA_MIPS_P8700_WFI
>> +#define ALT_WFI                                                              \
>> +ALTERNATIVE("wfi; .rept 7; nop; .endr;",                             \
>> +     ".rept 8; .insn 0x00501013; .endr;", MIPS_VENDOR_ID,            \
>> +     ERRATA_MIPS_P8700_WFI, CONFIG_ERRATA_MIPS_P8700_WFI)
> .insn only works on newer toolchains/binutils versions - this is why
> CONFIG_AS_HAS_INSN was added.  See for example commit
> dc20452e6caf962f04ede7f364267b0c37784ab4 ("riscv: Fix CONFIG_AS_HAS_INSN
> for new .insn usage").  Can you please fix this

Will fix this in v2!

>
> Please add some sort of code comment to describe why 8 MIPS PAUSE
> instructions are needed, as opposed to a single MIPS PAUSE instruction.
> An event that causes the first PAUSE to exit would then encounter the
> subsequent PAUSEs, at which point, there could be additional latency
> before the core can return to doing real work.  The P8700 Programmer's
> Guide mentions a maximum number of cycles per PAUSE instruction, but
> doesn't state what it is precisely.  Could you please add a comment here
> to describe what it is?

The number of PAUSE instructions was determined by testing on the Palladium
board configured with a P8700 CPU. Coremark performance was measured on
a one-core, two-harts setup. Coremark performance with eight PAUSE instructions
was equivalent to what was measured with WFI.

Kind regards,
Aleksa Paunovic