Bug 2540 - vm needs to clear the instruction cache on ppc
Status: RESOLVED FIXED
Alias: None
Product: ioquake3
Classification: Unclassified
Component: Platform
Version: 1.33 SVN
Hardware: Macintosh MacOS X
: P2 normal
Assignee: Zachary J. Slater
QA Contact: ioquake3 bugzilla mailing list
URL:
Depends on:
Blocks:
 
Reported: 2005-12-09 21:52 EST by Richard Lake
Modified: 2007-05-21 11:24:44 EDT
3 users (show)

See Also:

Attachments
updated macosx patch. (846 bytes, patch)
2005-12-16 15:32 EST, Ryan C. Gordon

Description Richard Lake 2005-12-09 21:52:59 EST 
The ppc vm appears to be missing a routine to clear the instruction cache when it's done generating code. For Mac OS X there is a MP-safe routine in CoreServices framework, patch against id source provided. I expect there's a similar call for linux or some code that could be used for all ppc targets.

--- quake3-1.32b.orig/code/qcommon/vm_ppc_new.c 2005-08-15 19:10:08.000000000 -0700
+++ quake3-1.32b/code/qcommon/vm_ppc_new.c      2005-12-09 18:15:21.000000000 -0800
@@ -24,6 +24,10 @@
 
 #include "vm_local.h"
 
+#ifdef MACOS_X
+#include <CoreServices/CoreServices.h>
+#endif
+
 #pragma opt_pointer_analysis off
 
 #define DEBUG_VM 0
@@ -1780,6 +1784,14 @@
            // go back over it in place now to fixup reletive jump targets
            buf = (unsigned *)vm->codeBase;
        }
+       else if ( pass == 1 ) {
+#ifdef MACOS_X
+           // On Mac OS X, the following library routine clears the instruction cache for generated code
+           MakeDataExecutable(vm->codeBase, vm->codeLength);
+#else
+#warning Need to clear the instruction cache for generated code
+#endif
+       }
     }
     if(0)
     {
Comment 1 Ryan C. Gordon 2005-12-16 10:03:25 EST 
zakk: please apply the Mac OS X patch; this will need work for PowerPC Linux, but the Mac patch looks good.

Notes to myself about how to do this on non-MacOS platforms:

Code from here:
http://publibn.boulder.ibm.com/doc_link/en_US/a_doc_lib/aixassem/alangref/sync.htm#HDRA28692BF

# Assume GPR 3 contains a modified instruction.
# Assume GPR 4 contains the address of the memory location
# where the modified instruction will be stored.
stw     3,0(4)           # Store the modified instruction.
dcbf    0,4              # Copy the modified instruction to
                         # main memory.
sync                     # Ensure update is in main memory.
icbi    0,4              # Invalidate block with old instruction.
isync                    # Discard prefetched instructions.
b       newcode          # Go execute the new code.


I don't know if this is well-behaved for SMP systems (but I'm fairly certain that Linux at least is pretty aggressive about cpu affinity, and forces a cache flush when it isn't).

At least "sync" and "icbi" are PowerPC only (so we'd have to detect that this isn't actually a POWER cpu to use them). "eieio" might be faster than "sync", too.

--ryan.
Comment 2 Zachary J. Slater 2005-12-16 13:39:59 EST 
Doesn't apply cleanly. And a comment isn't commented properly, even after fixing that however, it still doesn't apply.
Comment 3 Ryan C. Gordon 2005-12-16 15:32:18 EST 
Created attachment 824 [details]
updated macosx patch.


Zakk,

Here's a cleaned up version of the Mac OS X patch against the latest in svn. I'm not near a Mac to test it, so it's in your court. 

--ryan.
Comment 4 Tim Angus 2006-01-13 15:55:14 EST 
Committed the patch. Somebody with a Mac should now test the VM by re-enabling it (the reverse of bug 2519).
Comment 5 Julian Mayer 2006-01-13 18:06:14 EST 
>Committed the patch. Somebody with a Mac should now test the VM by re-enabling
>it (the reverse of bug 2519).

revision 477 still crashes wenn trying to scan for servers with the ppcnewvm
Comment 6 Tony J. White 2006-06-17 14:57:51 EDT 
No more reports of ppc vm crashes since bug 2519.

Closing this (if bugzilla lets me).
Comment 7 Ryan C. Gordon 2007-05-21 11:24:44 EDT 
Setting a QA contact on all ioquake3 bugs, even resolved ones. Sorry if you get a flood of email from this, it should only happen once. Apologies for the incovenience.

--ryan.