Page MenuHomeFreeBSD

riscv: handle page faults in dtrace_trap()
ClosedPublic

Authored by mhorne on Jan 31 2023, 7:26 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Jul 9, 10:27 PM
Unknown Object (File)
Fri, Jul 5, 11:39 PM
Unknown Object (File)
May 16 2024, 8:40 PM
Unknown Object (File)
May 16 2024, 8:40 PM
Unknown Object (File)
May 16 2024, 8:40 PM
Unknown Object (File)
May 16 2024, 8:40 PM
Unknown Object (File)
May 16 2024, 8:39 PM
Unknown Object (File)
May 14 2024, 8:46 PM
Subscribers

Details

Summary

We must detect the correct amount to increment sepc, as it may have been
a compressed instruction that triggered the fault.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

markj added inline comments.
sys/cddl/dev/dtrace/riscv/dtrace_subr.c
226–230

Here we assume 4-byte alignment, so I guess it's ok to assume that the faulting instruction won't be compressed?

227

It looks like this tests whether the low three bits of the instr are 0x1, but why does that work? It doesn't seem to line up with the MASK/MATCH_C constants defined in encoding.h.

This revision is now accepted and ready to land.Feb 1 2023, 3:43 PM
sys/cddl/dev/dtrace/riscv/dtrace_subr.c
226–230

Hmm, no, I was definitely seeing faults caused by compressed instructions, but misaligned access is emulated by M-mode firmware. I will reduce this to a uint16_t load.

227

*the low two bits

The instruction encoding space is set up so that all standard length instructions begin with the two lowest bits set to 1. Compressed instructions have a two-bit opcodes that can use any of the other patterns.

There isn't anything defined in encoding.h to precisely match for "any" compressed instruction. Essentially, the MASK definition for each specific instruction is too wide. This interface is a little obtuse, but is already used elsewhere in this file, so I tried to roll with it.

Still, I realize that I have got it wrong and this will only match some, but not all compressed instructions. It should become:

if (match_opcode(insn, 0x3, 0x3))
    frame->tf_sepc += INSN_SIZE;
else
    frame->tf_sepc += INSN_C_SIZE;

Fix logic of match_opcode(). Do a uint16_t load.

This revision now requires review to proceed.Feb 1 2023, 6:26 PM
This revision is now accepted and ready to land.Feb 2 2023, 2:28 PM