Generally this looks pretty good. I think your changes are correct except the way you dealt with t1 and t2 being switched in JMP_FAR_I. They're reversed like you said, but JMP_FAR_M and JMP_FAR_P and jmpFarWork match and should be left alone. t1 and t2 in JMP_FAR_I should be exchanged instead. JMP_FAR_M and JMP_FAR_P are using far pointers in memory, normally addressed in the first case and RIP relative in the second. They calculate the address of the start of the pointer and put it in t1, use that to find the selector which is farther into memory and put that in t2, and then finally overwrite t1 with the offset part of the pointer. I've historically had trouble with the ordering of the components of far pointers so you should probably double check that. I also suggest a temporary variable as described below.

Aside from the actual code, your commit message is good, but with mercurial the first line is considered a summary and will be displayed by itself in some circumstances. Your first line looks like a summary, but since there's a newline in the middle it'll get cut off. If that's just an artifact of review board then never mind.

Also, even though these are small and related changes, they're still separate changes that should probably go in separate patches and be committed separately. It's reasonable to put them all on review board together to make life easier for reviewers (aka me), but when these go in we'll want to split them up.

src/arch/x86/tlb.cc (Diff revision 1)

!entry->writable && (inUser || cr0.wp) is used here and in the if below. This would be easier to read if that was in a temporary variable, maybe called badWrite. Nate would probably yell at me that it should be bad_write because it's local. He'd be right, but that would be inconsistent with the existing code.