Page MenuHomeFreeBSD

Modify kernel MIPS assembly to conform to MIPS ABI convention of using $t9 as target $pc
Needs ReviewPublic

Authored by rwatson on Mar 6 2016, 12:17 PM.

Details

Reviewers
theraven
Group Reviewers
MIPS
Summary

This patch modifies the kernel's MIPS assembly to conform to the MIPS ABI convention of using $t9 as the target $pc of jalr (jump-and-link-register) instructions. This is a good idea generally, but is particularly helpful as Clang/LLVM's MIPS backend doesn't currently know how to generate code that doesn't depend on this ABI.

(Note: the floating-point code in fp.S is not updated by this patch.)

Test Plan

Tested locally on BERI/CHERI with the CHERI test suite using kernels compiled with both Clang/LLVM and in-tree gcc -- however, this is 64-bit MIPS only.

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

rwatson retitled this revision from to Modify kernel MIPS assembly to conform to MIPS ABI convention of using $t9 as target $pc.Mar 6 2016, 12:17 PM
rwatson updated this object.
rwatson edited the test plan for this revision. (Show Details)
rwatson added reviewers: MIPS, theraven.
rwatson set the repository for this revision to rS FreeBSD src repository.
rwatson updated this revision to Diff 14117.
adrian added a subscriber: adrian.Mar 10 2016, 5:08 PM

bug me after this weekend if I haven't tested this in qemu-mips (for 32 bit platforms.) I'm finally all setup to!

imp added a comment.Mar 10 2016, 5:16 PM

The whole t9 thing is new to me. I can't find where it's documented as being an actual thing. Can you help with that?

sys/mips/include/asm.h
243

Shouldn't we be using at for this? While this is panic, it does trash t9, where it didn't before.

251

Same.

sys/mips/mips/exception.S
544

why change this?
k0 I would think is the right register to use.

616

same.

676

same

741

same

863

same

sys/mips/mips/locore.S
205

Why isn't the jal op sufficient? It will get relocated correctly.

215

same

sys/mips/mips/mpboot.S
66

same

85

same

88

same

Hi Warner:

This has to do with "ABI calls" -- normally disabled in our kernel build using "-mno-abicalls" but currently unsupported by LLVM/MIPS. ABI calls provide a guarantee to callees that $t9 contains a pointer to the current function, relative to which other pointers to data can be calculated. This is documented in the System V Application Binary Interface supplement for MIPS:

http://math-atlas.sourceforge.net/devel/assembly/mipsabi32.pdf

See section 3-38. Although it is documented in the section on PIC, this use of $t9 is actually not normally limited to PIC code for MIPS, requiring us to pass a special compiler flag to disable it. Although this may cause an additional few instructions to be used in performing the initial JALR, the availability of $t9 can save instructions later (to my understanding) when performing jumps to shared objects, etc, and the compiler will normally rely on $t9 as a starting point for calculating pointers to other functions in the same module, and perhaps also for constant islands, etc -- much preferably to doing a JAL later to query an updated value of $pc.

Normally, the reason to use $k0/$k1 is that assembly code is executing in a context where treading on other registers (e.g., temporaries) would cause a state change in preempted code, as the context had not yet been fully saved. However, that is by definition not the case when we are about to call into a C-language kernel function, whose code will also assume that the temporaries are available to scribble all over in their own implementations. So it should always be safe to use $t9 when jumping to a C function.

rwatson added inline comments.Mar 11 2016, 9:25 AM
sys/mips/include/asm.h
243

There is no guarantee that code generation in panic() won't trash $t9 a few instructions later -- and, in fact, in normal MIPS code generation, panic() will trash $t9 as it calls vpanic(). If we want to preserve $t9 here, we need to trap so that a full frame is preserved.

251

Same here. Simply by calling printf(), we make it very likely that $t9 will be overwritten before anything is printed -- and countless other registers. If we want $t9 saved, then we need to capture a trap frame before calling a C function.

sys/mips/mips/exception.S
544

$k0 is generally used when working in a context where it is not OK to walk all over the register file. If we are able to safely call a compiler-generated function, then we know that register-file state has already be saved, as the compiler-generated code in a C function is guaranteed to walk all over the register file already.

imp added a comment.Mar 11 2016, 3:56 PM

Second pass. the only ones of these I'm worried about are the copyin/out ones and the extra instructions in the context switch path. All the others won't have any affect on performance.

sys/mips/include/asm.h
243

I was referring to the MACRO trashing t9, but yea, since we're calling it doesn't matter since jal jalr act as a fence for t* validity.

sys/mips/mips/exception.S
544

Yes, we've already saved the CPU state, so t9 is fine here.

616

This is also safe.

741

This is also safe, but the code here has changed with INTRNG

sys/mips/mips/locore.S
205

The ABI call here is OK, but annoying. At least it isn't in the critical path.

215

Same reply: OK. We're adding extra cycles to the startup, but only a few.

sys/mips/mips/mpboot.S
66

Adds cycles only on startup.

85

Adds cycles only on startup.

88

Adds cycles only on startup.

sys/mips/mips/support.S
190

This ads a cycle to the critical path code. as do all the others in this file. This will likely have a performance impact due to how often we're doing them.

sys/mips/mips/swtch.S
82

This adds a cycle to fork. Likely not a huge deal.

295

This adds a cycle to the context switch, but for saving the COP2 of the Octeon which is already going to be a heavyweight operation.

433

Also ads in instruction to the context switch.

theraven edited edge metadata.Mar 13 2016, 7:13 PM

While I agree that we need this in CheriBSD for experimentation with clang, I'd rather that we didn't put it in FreeBSD head unless we have a desire to make the kernel position independent (I've not looked at how we do modules on MIPS, so this may be a real requirement). On in-order implementations, the presence of the delay slot means that the jal can be resolved in time for instruction fetch to succeed, so we don't use a branch predictor table entry (and we won't accidentally alias with another branch in userspace somewhere). This is likely to have fairly complex performance implications on the variety of MIPS hardware available currently.

sbruno added a subscriber: sbruno.Sep 16 2016, 4:50 PM