Skip to content

Conversation

@LegacyNsfw
Copy link

See #1244

The "CPU32" iteration of the 68000 instruction set deprecates a few instructions and adds new table-lookup instructions. I I suspect this is not 100% complete, but it does a fine job on the firmware that I'm studying, so I don't plan to invest more time in it myself.

This is largely based on the work that @dzidaV8 began when that issue was opened. I could not have done this without that head start.

@backerman
Copy link

backerman commented Feb 2, 2021

This PR fails to build; certification.manifest needs an entry for CPU32.slaspec.

> Task :68000:ip FAILED

FAILURE: Build failed with an exception.

* Where:
Script 'C:\Users\bsa3\source\ghidra\gradle\support\ip.gradle' line: 125

* What went wrong:
Execution failed for task ':68000:ip'.
> No IP found for C:\Users\bsa3\source\ghidra\Ghidra\Processors\68000\data\languages\CPU32.slaspec in module: C:\Users\bsa3\source\ghidra\Ghidra\Processors\68000. Expression: (ip != null). Values: ip = null

@LegacyNsfw
Copy link
Author

Sorry for the slow turnaround on this, but I've added the missing entry, and I get a clean local build.

@GhidorahRex GhidorahRex self-assigned this Mar 9, 2021
# TODO: tbl_mode=4 and tbl_mode=5 constructors

tbl_eal: (tbl_regan) is tbl_mode=2 & tbl_regan { export *:4 tbl_regan; }
# tbl_eal: -(tbl_regan) is tbl_mode=4 & tbl_regan { tbl_regan = tbl_regan - 4; export *:4 tbl_regan; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were these commented out?

tbl_eal: -(tbl_regan) is tbl_mode=4 & tbl_regan { local tmp:4 = tbl_regan - 4; export *:4 tmp; }

Seems to work fine.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just don't have any actual code to prove that it works, and I didn't want to check in anything that wasn't tested. But since it was written, and might actually work, I left it there as a comment. So if anyone needs it they won't have to start from scratch.

That said, I don't feel strongly about commenting or not.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code commented out did not work, so I wouldn't suggest including it. I don't think we should not include the addressing modes however.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree it would be good to include them, but the binaries that I'm working with do not use them, so I don't have a way to test them.

I propose checking in what I have, as-is, because it works. When someone needs the additional modes I'll be happy to work with them to add it.

I think it would be a mistake to leave this in uncommitted until that day arrives.

What's in this pull request is a big step forward from the current code, and it will unblock anyone else who wants to reverse engineer the P01 and P59 family of General Motors LS V8 engine computers. I'll admit that's probably only a few people so far, but they were used pretty widely in 2001-2007 trucks (and 2001-2004 Corvettes) so I'm hoping more people will join in over time. It would be great to have Ghidra working as well for them as it does for me, right out of the box.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Go ahead and add them in with the fixes I suggested, I'll take care of testing the addressing modes.


tbl_eal: (tbl_regan) is tbl_mode=2 & tbl_regan { export *:4 tbl_regan; }
# tbl_eal: -(tbl_regan) is tbl_mode=4 & tbl_regan { tbl_regan = tbl_regan - 4; export *:4 tbl_regan; }
# tbl_eal: (d16,tbl_regan) is tbl_mode=5 & tbl_regan; d16 { local tmp = tbl_regan + d16; export *:4 tmp; }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbl_eal: (d16,tbl_regan) is tbl_mode=5 & tbl_regan; d16 { local tmp:4 = tbl_regan + d16; export *:4 tmp; }

Appears to work for this mode

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above. Since the code I'm working with doesn't need it, I couldn't be sure certain that it was correct.

If you'd rather I uncomment these, I'm perfectly fine with that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider renaming the commit to "Initial CPU32 support for 68k" or "Partial CPU32 support..." or something like that.

@GhidorahRex
Copy link
Collaborator

I'm still working on getting these pcodetests to run/pass. I've been running into a few issues. The first is on my end - gcc keeps inserting bfffo instructions which cpu32 doesn't know how to process.

The second is that the cpu32 pcodetests won't even run without at least one modification. The pcodeop countleadingzeroes needs to be moved out of the #ifndef CPU32 block and should be put around line 345 with the rest of the pcodeop definitions.

After that I'm still getting some failures that I'm looking into to see what the resolution should be.

:bclr.l reg9dn,regdn is op=0 & reg9dn & op68=6 & mode=0 & regdn { mask:4 = 1<<(reg9dn&31); ZF=(regdn&mask)==0; regdn=regdn&(~mask); }
:bclr.l d8,regdn is opbig=8 & op67=2 & mode=0 & regdn; d8 { mask:4 = 1<<d8; ZF=(regdn&mask)==0; regdn=regdn&(~mask); }

@ifndef CPU32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull out the define pcodeop countLeadingZeros line on 828 and move it up to the remainder of the define pcodeop lines in order to properly support emulation.

@backerman
Copy link

backerman commented Nov 5, 2023

I've manually rebased @LegacyNsfw's PR (so far without taking care of the comments) into a branch on my fork at backerman@fafd1bb. It mostly fails to apply because some previously unimplemented 68k instructions have since been implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature: Processor/68000 Status: Internal This is being tracked internally by the Ghidra team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants