Add BT/BTS/BTR/BTC instructions #122
Conversation
There was a problem hiding this comment.
Thank you for contributing this patch!
Have you tested theEDIT: Please ignore this.r/m, immvariant of these instructions (opcode0x0F 0xBA)? Because of the missingINSN_MODRMflag issue which I've pointed out in a review comment, I guess TempleOS only makes use of ther/m, rvariant.- On which host OS did you test this patch? I wonder if it was the same Linux host where you ran the KVM test. If so, I'm glad that our Linux port of HAXM (Added support for Linux hosts #108) is gaining popularity.
- In case you can also test this patch on Windows, could you write unit tests for the BTx instructions? We have a Google Test-based unit test framework for testing the instruction emulator (see
tests/test_emulator.cpp), but it can only be built on Windows at the moment. Or if you prefer, I can write the unit tests and push a second commit to this PR.
core/emulate.c
Outdated
| @@ -165,6 +165,14 @@ static const struct em_opcode_t opcode_group3[8] = { | |||
| F(em_neg, op_modrm_rm, op_none, op_none, 0), | |||
| }; | |||
|
|
|||
| static const struct em_opcode_t opcode_group8[8] = { | |||
| X4(N), | |||
| F(em_bt, op_modrm_rm, op_simm8, op_none, 0), | |||
There was a problem hiding this comment.
nit: Since we have already specified the same operand types in the opcode table (line 260), there's no need to repeat them here. So this line would become:
F(em_bt, op_none, op_none, op_none, 0),
and the same goes for the next 3 lines. In fact, opcode_group1 and opcode_group11 also follow this pattern.
Also nit: Just curious, how are the opcode groups numbered? I haven't been able to make sense of these numbers (1, 3, 8, 11, ...).
There was a problem hiding this comment.
Just curious, how are the opcode groups numbered? I haven't been able to make sense of these numbers (1, 3, 8, 11, ...).
This comes from the Intel SDM Vol. 2D, specifically:
- Groups 1, 3 and 11: Table A-2. One-byte Opcode Map: (00H — F7H)
- Group 8: Table A-3. Two-byte Opcode Map: 88H — FFH (First Byte is 0FH)
There was a problem hiding this comment.
Wow, thanks! This is the first time I've opened Vol. 2D...
@raphaelning His message mentioned being on Linux 4.18.10, so if Windows is unavailable to him, I could take care of the tests: I wanted to add few extra cases this weekend anyway. |
|
@AlexAltea Sure, thanks! I do see the mention of Linux, but I'm not sure if it was only used for the KVM test (as a "control group"). |
They were already implemented as fastops, so just add them to the opcode table. Signed-off-by: Jan Erik Petersen <JanErikPetersen@web.de>
|
Thanks for the fast review! I made the requested change and force-pushed it.
It was indeed the same Linux machine. Alex' linux port actually is what made me try out haxm.
Unfortunately, my experience with x86 is very limited and I'm not sure I could write meaningful tests, so I'd really appreciate it if either of you could implement them. |
Sure! As soon as this is merged, I'll prepare the tests for it! |
|
Thank you both! We'll test this PR and get it merged. |
Bit Test is used by TempleOS. With this change QEMU+HAXM are able to boot, install and run TempleOS.
Running the integrated test suite, 170/171 tests work without any problems. One test crashes with a General Protection Fault, but this happens on KVM too, so I don't think it's haxms fault. (Tested on an i3-380M running Linux Kernel 4.18.10 x86_64)
Signed-off-by: Jan Erik Petersen JanErikPetersen@web.de