riscv: Handle supervisor instruction page faults
ClosedPublic
Actions

Authored by jrtc27 on Oct 5 2020, 6:27 PM.

Details

Reviewers

jhb
markj

Group Reviewers

riscv

Commits

rS366486: riscv: Handle supervisor instruction page faults

Summary

We should never take instruction page faults when in the kernel, but by
using the standard page fault code we should get a more-informative
message about faulting on a NOFAULT page rather than branching to the
default case here and printing an "Unknown kernel exception ..."
message.

Diff Detail

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 34002
Build 31191: arc lint + arc unit

Event Timeline

jrtc27 requested review of this revision.Oct 5 2020, 6:27 PM

jrtc27 created this revision.

Harbormaster completed remote builds in B34002: Diff 77906.Oct 5 2020, 6:27 PM

Minor typo in commit log: s/shold/should/

This revision is now accepted and ready to land.Oct 5 2020, 7:54 PM

jrtc27 edited the summary of this revision. (Show Details)Oct 5 2020, 7:55 PM

Somewhat related, Wes from Cambridge had noted a while back that if we get a fault due to a SUM violation, we will just keep retrying the fault endlessly instead of panicking. That check probably belongs somewhere in the page_fault_handler function.

In D26685#594577, @jhb wrote:

Somewhat related, Wes from Cambridge had noted a while back that if we get a fault due to a SUM violation, we will just keep retrying the fault endlessly instead of panicking. That check probably belongs somewhere in the page_fault_handler function.

Isn't that handled by the pcb->pcb_onfault == 0 check in the !usermode case?

In D26685#594762, @markj wrote:

In D26685#594577, @jhb wrote:

Somewhat related, Wes from Cambridge had noted a while back that if we get a fault due to a SUM violation, we will just keep retrying the fault endlessly instead of panicking. That check probably belongs somewhere in the page_fault_handler function.

Isn't that handled by the pcb->pcb_onfault == 0 check in the !usermode case?

I _think_ the concern was if you have a non-zero pcb_onfault (e.g. you forget to clear it) but SUM is clear and try to access a valid userspace address then pmap_fault_fixup or vm_fault_trap will succeed and you will keep retrying the instruction.

Closed by commit rS366486: riscv: Handle supervisor instruction page faults (authored by jrtc27). · Explain WhyOct 6 2020, 1:03 PM

This revision was automatically updated to reflect the committed changes.

jrtc27 added a commit: rS366486: riscv: Handle supervisor instruction page faults.

Herald added a subscriber: imp. · View Herald TranscriptOct 6 2020, 1:03 PM

In D26685#594767, @jrtc27 wrote:

In D26685#594762, @markj wrote:

In D26685#594577, @jhb wrote:

Somewhat related, Wes from Cambridge had noted a while back that if we get a fault due to a SUM violation, we will just keep retrying the fault endlessly instead of panicking. That check probably belongs somewhere in the page_fault_handler function.

Isn't that handled by the pcb->pcb_onfault == 0 check in the !usermode case?

I _think_ the concern was if you have a non-zero pcb_onfault (e.g. you forget to clear it) but SUM is clear and try to access a valid userspace address then pmap_fault_fixup or vm_fault_trap will succeed and you will keep retrying the instruction.

I do think we should validate the SUM bit in page_fault_handler(). I just assumed that someone had actually managed to trigger this bug and was a bit surprised since SSTATUS_SUM and pcb_onfault are always(?) manipulated together.

Revision Contents
Changeset List

Path

Size

sys/

riscv/

trap.c

1 line

Diff 77906

View Options

riscv: Handle supervisor instruction page faultsClosedPublicActions