Page MenuHomeFreeBSD

Unify Book-E and AIM trap.c
ClosedPublic

Authored by jhibbits on Apr 3 2015, 1:50 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Mar 30, 4:22 AM
Unknown Object (File)
Jan 8 2024, 9:29 PM
Unknown Object (File)
Jan 8 2024, 9:29 PM
Unknown Object (File)
Jan 8 2024, 9:13 PM
Unknown Object (File)
Jan 4 2024, 10:45 AM
Unknown Object (File)
Dec 22 2023, 9:26 PM
Unknown Object (File)
Nov 10 2023, 9:16 AM
Unknown Object (File)
Nov 5 2023, 5:01 PM
Subscribers

Details

Summary

Book-E and AIM trap.c are almost identical, except for a few bits. This is step
1 in unifying them. There are two things of note:

  1. booke setfault() also saves CTR and XER, which AIM does not. I don't know if

it should, or if booke should not be saving CTR and XER, so I want opinions as
the best way to do it.

  1. Some of the exception codes overlap between AIM and booke. I commented out

the EXC_RUNMODETRC case statement, because that exception code conflicts with
EXC_DEBUG for booke, and EXC_RUNMODETRC is documented as only on the ppc601,
which we do not support. However, since Book-E codes are entirely arbitrary
(using IVORs instead of fixed address offsets, which account for the exception
codes for AIM), I'd like to renumber them, so soliciting opinions on that as
well.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

jhibbits retitled this revision from to Unify Book-E and AIM trap.c.
jhibbits updated this object.
jhibbits edited the test plan for this revision. (Show Details)
jhibbits added reviewers: PowerPC, nwhitehorn, marcel.
marcel edited edge metadata.

In general: looks good.

Ad 1:
Since setfault is in locore.S and you're not merging that code, I would defer this question until later. That said: I don't know whether either of the Book-E or AIM functions is optimized and precisely save the minimal set of registers or whether they're still relatively quick-n-dirty so so that it works.
All they need to do is signal to the trap handler, when it handles a page fault, that user space is accessed and that control should return to the function doing the access with enough state saved that this can be done without violating the ABI. Thus the question mostly pivots around what registers need to be preserved across calls (and consequently what registers can be considered scratch) and what the user-space accessor function saves extra to deal with the trap.
Put differently: they are what the comment says; very similar to setjmp. See what setjmp saves and why and you should have a good starting point. Note that setfault is rather special-purpose, so it's possible it can save less.
Most/some architectures do not need to save anything. They just need to set a flag and an instruction address to return to from interrupt. You may want to ask yourself why PowerPC uses a setjmp approach that requires saving registers in all cases instead of a lazy let-it-trap approach that only requires saving registers when a fault happens... There's room for a noticeable performance improvement by letting the standard trap code save and restore registers as needed...

Ad 2:
Since Book-E uses arbitrary codes, it makes sense to match these codes to the vector addresses of AIM. Exceptions specific to Book-E can use codes that is not likely to ever be used: something larger than EXC_LAST for AIM.

This revision is now accepted and ready to land.Apr 4 2015, 4:25 PM

Looks good in general. One comment regarding Marcel's comment 2: I think all the codes are already unified, actually.

sys/powerpc/powerpc/trap.c
207

What's going on here with the commented-out entry?

Closed by r281096/281097.