Re: [PATCH v3 6/7] riscv: add a data fence for CMODX in the kernel mode

[Björn Töpel]

Andy Chiu <andybnac@xxxxxxxxx> writes:

> RISC-V spec explicitly calls out that a local fence.i is not enough for
> the code modification to be visble from a remote hart. In fact, it
> states:
>
> To make a store to instruction memory visible to all RISC-V harts, the
> writing hart also has to execute a data FENCE before requesting that all
> remote RISC-V harts execute a FENCE.I.
>
> Thus, add a fence here to order data writes before making the IPI.
>
> Signed-off-by: Andy Chiu <andybnac@xxxxxxxxx>
> ---
>  arch/riscv/mm/cacheflush.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/mm/cacheflush.c b/arch/riscv/mm/cacheflush.c
> index b81672729887..b2e4b81763f8 100644
> --- a/arch/riscv/mm/cacheflush.c
> +++ b/arch/riscv/mm/cacheflush.c
> @@ -24,7 +24,20 @@ void flush_icache_all(void)
>  
>  	if (num_online_cpus() < 2)
>  		return;
> -	else if (riscv_use_sbi_for_rfence())
> +
> +	/*
> +	 * Make sure all previous writes to the D$ are ordered before making
> +	 * the IPI. The RISC-V spec states that a hart must execute a data fence
> +	 * before triggering a remote fence.i in order to make the modification
> +	 * visable for remote harts.
> +	 *
> +	 * IPIs on RISC-V are triggered by MMIO writes to either CLINT or
> +	 * S-IMSIC, so the fence ensures previous data writes "happen before"
> +	 * the MMIO.
> +	 */
> +	RISCV_FENCE(w, o);

(I love the submit/review latency here! ;-))

FWIW, the for S-IMSIC the write is already writel(), so we'll have the
text patching and IPI ordered. Regardless, there's more than one flavor
of IPI on RISC-V!

Reviewed-by: Björn Töpel <bjorn@xxxxxxxxxxxx>

> +	if (riscv_use_sbi_for_rfence())
>  		sbi_remote_fence_i(NULL);
>  	else
>  		on_each_cpu(ipi_remote_fence_i, NULL, 1);
> -- 
> 2.39.3 (Apple Git-145)