RFR: 8310656: RISC-V: __builtin___clear_cache can fail silently.

[Robbin Ehn]

On Wed, 28 Jun 2023 05:33:11 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

> Got curious. Looks good.
> 
> I was not even aware that __builtin___clear_cache had an errno. Is this documented somewhere? We should probably check arm64 too.
> 
> Question, would cacheflush(2) have worked too? Instead of the syscall?

Hi Thomas, thanks for looking, and thanks for being curios!

And sorry in advanced for this long answer, I felt that I needed to add some context.

Cross modifying code/CMODX is not specified on RV.
RV allows both coherent and non-coherent I/D cache.
Right now we have to assume non-coherent I/D cache (I have suggest hwprobe should return this).

But there is two pieces we can use, ifence and full IPI shootdown.
ifence is a local hart instruction read fence.
With non-coherent I/D cache ifence is not sufficient.
And ifence is an extension, not on all RV cpus.

Kernel 4.15 got the syscall riscv_flush_icache added with the IPI shootdown.
There is also a hart local version SYS_RISCV_FLUSH_ICACHE_LOCAL, which can be used instead of ifence.

If we ignore the case of having an UP (as you know we removed UP from the VM),
to make sure all threads/harts have a fresh I-cache our only option, at the moment,
is to emit that syscall => IPI.

There is still some cases in the VM I believe is not entirely correct.
The IPI is very expensive and system-wide intrusive, so if we are running on
I/D cache coherent system we should be able to utilize the ifence instruction and avoid (some?) IPI's.

Regarding the cacheflush wrapper it does not seem to implemented and if I tested it I get undef ref when linking. I only see the syscall in that header.

It looks like glibc/gcc don't consider failing syscalls, maybe this was uncommon, but now when everyone is running in different containers it's certainly more common. E.g. cacheflush do not list EPERM as a return value.

Note that there is a vdso wrapper for this syscall, which just do the ecall (syscall instruction), so I fail to see the point in trying to use that. And since we are doing an a full system IPI the speed of the syscall is not essential.

So I feel more comfortable with the VM explicit emitting this syscall, so everyone can see exactly what we are doing.

> src/hotspot/os_cpu/linux_riscv/riscv_flush_icache.cpp line 44:
> 
>> 42: 
>> 43: #define assert_with_errno(cond, msg)    check_with_errno(assert, cond, msg)
>> 44: #define guarantee_with_errno(cond, msg) check_with_errno(guarantee, cond, msg)
> 
> useful; potentially in debug.hpp?

I'll create an jira ticker, I have spread this around to other places which also needs to be cleanup.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/14670#issuecomment-1610966486
PR Review Comment: https://git.openjdk.org/jdk/pull/14670#discussion_r1244854484