Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/ppcvletoolchain #338

Open
wants to merge 14 commits into
base: master
Choose a base branch
from

Conversation

paulnoalhyt
Copy link
Collaborator

One sentence summary of this PR (This should go in the CHANGELOG!)
Support for PPC VLE in patchmaker and Ghidra.

Link to Related Issue(s)
#238

Please describe the changes in your request.
This pull request's goal is to add support for PPC VLE in different OFRAK components:

  • definition of the VLE sub-isa
  • Installation of the toolchain in the patchmaker Dockerfile. Support of this toolchain in the patchmaker and automated tests for it
  • Support for VLE analysis in Ghidra and fixups in the instruction unpacker

Anyone you think should look at this, specifically?
@rbs-jacob

@rbs-jacob
Copy link
Member

Instead of #341 for allowing secrets (which may or may not work), maybe we can just mark the PPC VLE stuff with # pragma: no cover and not be blocked on that?

Test coverage is all well and good, but I don't know how much we want to block on it for this. There will still be test coverage after merge on the master branch (where secrets will work), just not on PR branches from external contributor forks.

ppc_vle = True
break
if ppc_vle:
return SubInstructionSet.PPCVLE
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not 100% sold on this representation of VLE. We already have it as a "mode" similar to Thumb. Similar to Thumb, the whole binary is not necessarily VLE. More info here: https://www.nxp.com/docs/en/engineering-bulletin/EB687.pdf

The VLE extension may be used globally within an application, or applied only to specific sections of the application. The e200z0 core is an exception, it supports only the VLE instruction set.

We could consider it as both a "mode" and a sub-isa because it sort of acts like both. But maybe we should only consider the binary to have the sub-isa of "PPCVLE" if ALL of the (executable) sections/segments are VLE. Otherwise, it's normal PPC, with some areas that are PPC VLE.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I chose to use a SubInstructionSet for several reasons:

  • I've never seen an actual target running both VLE and normal PPC instructions. Even though, indeed, the mode can be set per page of memory (https://www.nxp.com/docs/en/supporting-information/VLEPIM.pdf):

    This alternate encoding set is selected on an instruction page basis. A single page attribute bit selects between standard PowerPC Book E instruction encodings and the VLE instructions for the particular page of memory. This page attribute is an extension to the existing PowerPC Book E page attributes. Pages can be freely intermixed, allowing for a mixture of code with both types of encodings

  • tagging a whole executable as PPC VLE, we can force Ghidra to analyze everything as VLE instructions (it has a tendency to prefer decoding normal PPC instructions when it can)
  • even on a target whare all instructions are VLE, not all executable sections and segments would have this VLE flag

Copy link
Collaborator

Choose a reason for hiding this comment

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

Are there binaries with executable code in a section which is not marked with that VLE flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've only seen ELF files where all code sections are PPC VLE. But I think that in theory an ELF file could contain both PPC and PPC VLE code.

@EdwardLarson EdwardLarson marked this pull request as draft July 6, 2023 18:55
@paulnoalhyt paulnoalhyt marked this pull request as ready for review July 10, 2023 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants