search

intel / processwatch

GNU General Public License v2.0
121 stars 9 forks source link

Support for Arm #15

Closed grahamwoodward closed 2 months ago

grahamwoodward commented 4 months ago

Hi I was wondering whether you'd accept a patch that gets Process Watch working on Arm, specifically targeting aarch64?

I have a patch ready to go but wanted to first check whether it's something you'd be willing to accept, or if you prefer I forked the repo ?

Thanks

matthew-olson-intel commented 3 months ago

Of course it's something I'd accept, y'know, assuming it's nothing too crazy. I'm assuming it's just an extra condition checking the value of pmu_name, and some extra logic choosing a perf event to use, right?

Edit: Forgot about disassembly. Is there some disassembler that can handle both x86 and ARM?

grahamwoodward commented 3 months ago

Of course it's something I'd accept, y'know, assuming it's nothing too crazy. I'm assuming it's just an extra condition checking the value of pmu_name, and some extra logic choosing a perf event to use, right?

Edit: Forgot about disassembly. Is there some disassembler that can handle both x86 and ARM?

So to answer the second question, I've added a dependency on Capstone, it actually supports x86...however I didn't really want to be on the hook for regression testing x86 if we switched to Capstone, although it'd make my patch a bit neater as I could remove some #ifdefs that I've got depending on whether it's compiling for x86 or Arm.

And yeah for the first question, I'm not checking for the pmu_name and using a slightly different event id. Also needed to change the default "categories"

How about I raise a PR and you can see. I'm happy to look at swapping Zydis for Capstone as well, do you have any regression tests internally that do any checking that certain x86 instructions have been correctly decoded that we could use to confirm all is good with Capstone?

matthew-olson-intel commented 3 months ago

Sure thing, submit the PR. Capstone could be a good idea; I'll give it a spin.

grahamwoodward commented 3 months ago

@matthew-olson-intel I've opened a PR with the patch...admittedly the code is a littered with #ifdef's, which isn't the best...we could either

  1. Replace Zydis with Capstone

  2. Add another layer of abstraction and some APIs to get the mnemonics/categories and have those APIs call the right disassembler

  3. would remove some of the #ifdefs but 1. is probably best and would allow PW to support other architectures

matthew-olson-intel commented 2 months ago

Aight, this is in progress in #17.

matthew-olson-intel commented 2 months ago

Fixed by #17!