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

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

Merged
merged 3 commits into from
May 14, 2023
Merged

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

merged 3 commits into from
May 14, 2023

Conversation

tkchia
Copy link
Collaborator

@tkchia tkchia commented May 12, 2023

No description provided.

@tkchia tkchia changed the title Fix smsw & lmsw instructions; implement lss, lfs, lgs Fix push, pop, smsw, lmsw; implement lss, lfs, lgs May 13, 2023
@tkchia tkchia marked this pull request as ready for review May 13, 2023 08:43
The stack address size (whether to use %sp, %esp, or %rsp)
is fixed by the segment descriptors in effect; & cannot be
altered by 0x66 or 0x67 prefixes.  Thus it is more correct
to use Mode(rde) (versus Eamode(rde)) to compute the stack
address size.
@ghaerr
Copy link
Collaborator

ghaerr commented May 13, 2023

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
Copy link
Collaborator Author

tkchia commented May 13, 2023

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. 😬

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. 🙂

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
Copy link
Collaborator Author

tkchia commented May 13, 2023

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
Copy link
Collaborator Author

tkchia commented May 14, 2023

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

@ghaerr
Copy link
Collaborator

ghaerr commented May 14, 2023

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 tkchia merged commit 72f0841 into jart:master May 14, 2023
@tkchia
Copy link
Collaborator Author

tkchia commented May 14, 2023

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!

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.

2 participants