Re: [PATCH RFC v2 riscv/for-next 0/5] Enable ftrace with kernel preemption for RISC-V

From: Samuel Holland <samuel.holland@sifive.com>
To: Alexandre Ghiti <alex@ghiti.fr>, Andy Chiu <andy.chiu@sifive.com>
Cc: Evgenii Shatokhin <e.shatokhin@yadro.com>,
	palmer@dabbelt.com, paul.walmsley@sifive.com,
	aou@eecs.berkeley.edu, rostedt@goodmis.org, mingo@redhat.com,
	peterz@infradead.org, jpoimboe@kernel.org, jbaron@akamai.com,
	ardb@kernel.org, greentime.hu@sifive.com, zong.li@sifive.com,
	guoren@kernel.org, Jessica Clarke <jrtc27@jrtc27.com>,
	kernel@esmil.dk, linux-riscv@lists.infradead.org,
	linux@yadro.com
Subject: Re: [PATCH RFC v2 riscv/for-next 0/5] Enable ftrace with kernel preemption for RISC-V
Date: Thu, 7 Mar 2024 09:57:28 -0600	[thread overview]
Message-ID: <c1d994e2-8f0c-4039-b790-a426fc48055c@sifive.com> (raw)
In-Reply-To: <f4506e4f-fca5-49ac-868e-f42f1f23ce9b@ghiti.fr>

Hi Alex,

On 2024-03-07 7:21 AM, Alexandre Ghiti wrote:
> But TBH, I have started thinking about the issue your patch is trying to deal
> with. IIUC you're trying to avoid traps (or silent errors) that could happen
> because of concurrent accesses when patching is happening on a pair auipc/jarl.
> 
> I'm wondering if instead, we could not actually handle the potential traps:
> before storing the auipc + jalr pair, we could use a well-identified trapping
> instruction that could be recognized in the trap handler as a legitimate trap.
> For example:
> 
> 
> auipc  -->  auipc  -->  XXXX  -->  XXXX  -->  auipc
> jalr        XXXX        XXXX       jalr       jalr
> 
> 
> If a core traps on a XXXX instruction, we know this address is being patched, so
> we can return and probably the patching will be over. We could also identify
> half patched word instruction (I mean with only XX).

Unfortunately this does not work without some fence.i in the middle. The
processor is free to fetch any instruction that has been written to a location
since the last fence.i instruction. So it would be perfectly valid to fetch the
old aiupc and new jalr or vice versa and not trap. This would happen if, for
example, the two instructions were in different cache lines, and only one of the
cache lines got evicted and refilled.

But sending an IPI to run the fence.i probably negates the performance benefit.

Maybe there is some creative way to overcome this.

> But please let me know if that's completely stupid and I did not understand the
> problem, since my patchset to support svvptc, I am wondering if it is not more
> performant to actually take very unlikely traps instead of trying to avoid them.

I agree in general it is a good idea to optimize the hot path like this.

Regards,
Samuel

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv