jart / blink

tiniest x86-64-linux emulator
ISC License
7k stars 224 forks source link

Fix `push`, `pop`, `smsw`, `lmsw`; implement `lss`, `lfs`, `lgs` #133

Closed tkchia closed 1 year ago

ghaerr commented 1 year ago

Hello @tkchia,

Nice work! I'm wondering how you're finding these bugs - in particular, the latest finding that an address size prefix is ignored on push/pop. It kind of reminds of of when, on a much earlier version of blink, the real mode ELKS boot ran for a while, then crashed when a (%bp)-accessed parameter wasn't at the proper stack address. That took a while a fine. Turned out that blink was running 8-byte push operations instead of 2. What was kind of amazing is how far the boot process ran in the first place!

It looks like most of these changes are working towards getting much more of the protected mode emulation implemented?

Thank you!

tkchia commented 1 year ago

Hello @ghaerr,

Actually I spotted the push and pop bug due to a somewhat unrelated issue while trying to implement support for the single-step interrupt (TF = 1 ⇒ int 1). I was trying out Blink on the Second Reality startup code, as I hinted over at Discord. :grimacing:

The single-stepping code is mostly working now (https://github.com/tkchia/blink/commit/efef93ce28baa47850b63f6e67c613bd0e10147a), but I probably need to test it further and resolve any remaining loose ends.

Incidentally, the Second Reality code also quite directly triggered the bug in the smsw emulation, which is how I found that bug. :slightly_smiling_face:

It looks like most of these changes are working towards getting much more of the protected mode emulation implemented?

That is something I hope Blink can achieve some time, if space allows (?). One big chunk of functionality that protected mode OSes — e.g. Coherent, Xenix — seem to want, is support for 16-/32-bit task state segments (ltr). This is not trivial to do.

Thank you!

tkchia commented 1 year ago

Hello @ghaerr,

Also, from what I understand, actually the address size prefix is not exactly ignored on push/pop. If you push or pop a memory operand then the presence or absence of 0x67 will determine the address size. So you can distinguish between, say, pushq (%ecx) and pushq (%rcx). But the prefix will not change whether %ss:%esp or %ss:%rsp is used for the stack pointer in the push.

Thank you!

tkchia commented 1 year ago

Do confirm if this is OK to merge. Thank you!

ghaerr commented 1 year ago

Hello @tkchia,

LGTM. Are there test cases for testing the difference between Eamode and Mode for the more regularly-used case of long mode emulation and running of larger binaries like gcc? I would think that most of the changes you're making for bare metal and/or protected mode aren't a big deal, since far fewer people use those features, unless they might also affect long mode execution. There may already be several, but having a test case for regression testing long mode execution of Linux binaries would ensure that our bare metal contributions don't break that. (I have been less worried about this for my contributions since I have stayed away from the internals of the x86_64 CPU emulation).

Thank you!

tkchia commented 1 year ago

Hello @ghaerr,

LGTM. Are there test cases for testing the difference between Eamode and Mode for the more regularly-used case of long mode emulation and running of larger binaries like gcc?

Thanks! I do not think there are any such test cases yet, but these should not be too hard to come up with.

In this PR, I am already extending an existing test program test/asm/push.S which is meant to be run under either Linux/x86-64 or bare metal. So perhaps we could extend it even further, to map some memory in the 32-bit space, and then test whether popq (%ecx) works, etc. Alternatively we can add new test programs to test/asm/ or test/func/.

Thank you!