Skip to content
This repository has been archived by the owner on Jan 28, 2023. It is now read-only.

Exception interface for supervisor #199

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

Conversation

leecher1337
Copy link
Contributor

When implementing HAXM into NTVDMx64, I experienced the need to tell HAXM some exit NMI vectors that I needed to handle (for instance, handle an invalid exception to be able to handle BOPs by client code).
Therefore I would consider it useful to have such an interface.

…TER_GS, is it VMX_EXIT_TRIPLE_FAULT? We want to know!
… to supervisor. This is i.e. needed for BOPping in NTVDM (VECTOR_UD) or for Interrupt hooking (VERTOR_NP).

So this generic call could be useful for hypervisors.
@krytarowski
Copy link
Contributor

Sorry, but a lot of whitespace issues (tabs vs spaces).

@HaxmCI HaxmCI added CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels Apr 19, 2019
core/memslot.c Outdated
memslot_init(dest, src);
dest->entry = entry;
Copy link
Contributor

Choose a reason for hiding this comment

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

Many thanks for @leecher1337 shooting the root cause! For performance reasons, it is recommended to get rid of memslot_init() invocation to avoid assigning dest->entry 3 times within this function. I drafted the commit message for your reference. The following code should be equivalent for your reference. Thanks again.

Fix a bug of memslot_move()

Invoking memslot_init() in memslot_move() will result in losing the
linked list information of the original memory slot node. Reimplement
memslot_move() to resolve some exceptional issues during mapping memory
slots.

static inline void memslot_move(hax_memslot *dest, hax_memslot *src)
{
ramblock_deref(dest->block);
src->entry = dest->entry;
*dest = *src;
ramblock_ref(dest->block);
}

…sign considerations for calling memslot_init
core/memslot.c Outdated
dest->entry = entry;
static inline void memslot_move(hax_memslot *dest, hax_memslot *src)
{
ramblock_deref(dest->block);
Copy link
Contributor

Choose a reason for hiding this comment

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

According to the current coding style, it is recommended to use 4 spaces as indent.

Bugfix description:
Invoking memslot_init() in memslot_move() will result in losing the
linked list information of the original memory slot node. Reimplement
memslot_move() to resolve some exceptional issues during mapping memory
slots.
core/cpu.c Outdated
@@ -222,6 +222,7 @@ bool vcpu_is_panic(struct vcpu_t *vcpu)
if (vcpu->panicked) {
hax_error("vcpu has panicked, id:%d\n", vcpu->vcpu_id);
hax_panic_log(vcpu);
htun->_exit_reason = vmx(vcpu, exit_reason).basic_reason;

Choose a reason for hiding this comment

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

use spaces

Suggested change
htun->_exit_reason = vmx(vcpu, exit_reason).basic_reason;
htun->_exit_reason = vmx(vcpu, exit_reason).basic_reason;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, hopefully this was the last tab vs. space issue, I did a grep on \t now and didn't find any more areas

@wcwang
Copy link
Contributor

wcwang commented Aug 28, 2019

@leecher1337, we are preparing a new release and this pull request #199 needs more time to review from my colleagues. Do you agree to submit a separate pull request for memslot.c to fix the memslot_move() bug? We would like to merge this patch first in this release. The below title and content of commit message are for your reference. And remember to add 'Signed-off-by' signature in commit message. Thanks.


memslot: Fix a bug of memslot_move()

Invoking memslot_init() in memslot_move() will result in losing the
linked list information of the original memory slot node. Reimplement
memslot_move() to resolve some exceptional issues during mapping memory
slots.

@HaxmCI HaxmCI added CI:Build Fail CI:Build Fail and removed CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels Aug 28, 2019
@HaxmCI HaxmCI added CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass and removed CI:Build Fail CI:Build Fail labels Aug 29, 2019
@coxuintel
Copy link
Contributor

@leecher1337 I just found your amazing project https://github.com/leecher1337/ntvdmx64 . Can I assume that you are adding VT support using HAXM? Besides the README in the project, is there any additional step I should take to test you HAXM implementation? I'd like to test NTVDMx64 and check the HAXM issue you mentioned.

core/vcpu.c Outdated
void vcpu_setexcbmp(struct vcpu_t *vcpu, uint32_t excbmp)
{
vcpu->user_excbmp = excbmp;
hax_log(HAX_LOGE, "set user_excbmp = %08X", vcpu->user_excbmp);

Choose a reason for hiding this comment

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

tab -> spaces

…e spaces instead of tabs on a PER-PROJECT basis? (EditorConfig possibly?)
@leecher1337
Copy link
Contributor Author

@coxuintel
You are right,it's exactly the project I'm using HAXM for.
I hopefully will find time to release a patch-set in the next days,so that HAXM support can be tried out, so watch for commits.
Be aware that there are currently some issues preventing satisfactory operation:

  • Currently, only realmode works with HAXM, DPMI will crash (investigating this will be tough, I need to find a way to hook interrupts for instance,which is required for DPMI, some musings about it can be found in the HAXM support sourcecode files already)
  • Permit MMIO exits to bypass the emulation. #164 is still a show-stopper. While I get decent speed-ups in one-page Textmode applications and DOS applications using streaming I/O compared to the CCPU, everything involving a lot of video output will suffer from a huge performance degradation. Original NTVDM was able to use the VGA card directly to prevent performance problems with applications doing graphics with the XPDM Display driver model. However as of Windows 8, XPDM was discontinued and replaced wirth WDDM which doesn't support VGA at all (Original Windows 32bit NTVDM suffers from the same problem since Win8). So NTVDMx64 can only do graphics by using the console screen buffer (yes, it still exists on Windows, it just has a bug that my loader fixes, see Bringing back the console graphics screen-buffers? microsoft/terminal#246 ). FASTMMIO is far too slow for it and the method I choose as a workaround doesn't support VGA planar reads. I think that this is an architectural problem that cannot be solved, as qemu etc. have the same issues.
  • I currently didn't check in all the modifications that I made to HAXM so that this pull request won't get polluted by the other changes. I may need to add a new branch for that.

@coxuintel
Copy link
Contributor

coxuintel commented Sep 9, 2019

For the rendering performance issue, XPDM or WDDM may not be the root cause from my point of view. In Vista or Win7, WDDM is the default graphics driver model. Unless you force install a XPDM driver onto the graphics adapter, the GPU works in WDDM model. So it may be due to the removed support of video BIOS INT call or something else, but not the XPDM or WDDM difference. I'm thinking, if it's possible to "translate" the real mode drawing to simply memory write, then use Direct2D or Direct3D to blit the content to expected place so that you can leverage GPU HW acceleration.
btw. Have you checked how DOS run on QEMU+KVM?

@leecher1337
Copy link
Contributor Author

leecher1337 commented Sep 11, 2019

For the rendering performance issue, XPDM or WDDM may not be the root cause from my point of view. In Vista or Win7, WDDM is the default graphics driver model. Unless you force install a XPDM driver onto the graphics adapter, the GPU works in WDDM model. So it may be due to the removed support of video BIOS INT call or something else, but not the XPDM or WDDM difference. I'm thinking, if it's possible to "translate" the real mode drawing to simply memory write, then use Direct2D or Direct3D to blit the content to expected place so that you can leverage GPU HW acceleration.

The removal of the ability to pass through the MMIO area to the real video adapter (possibly, because all rendering is now done via the GPU und therefore switch to VGA mode isn't supported) removed the drawing possibilities of NTVDM.
As you can clearly see, you cannot use graphics on normal 32bit NTVDM anymore. Try to do ALT+RETURN key combination and you will get an error message.
See https://docs.microsoft.com/en-us/windows-hardware/drivers/ddi/content/video/nf-video-videoportsettrappedemulatorports for more details. As long as there is no possibility to directly access the VGA card, I don'T see any change for usable video output in V86/VT-x.

btw. Have you checked how DOS run on QEMU+KVM?

Yes, it's completely unusable in this regard as soon as QEMU uses HAXM. Without HAXM with the emulated CPU, it's OK.
dosemu switches to an emulated CPU to circumvent the problem and then switches back to hardware after a certain numer of instructions got executed without any video access.

@leecher1337
Copy link
Contributor Author

leecher1337 commented Sep 11, 2019

@coxuintel HAXM-build of NTVDMx64 is now available. Just check doc\haxm.txt for details.
Basically you have to use bld-haxm.cmd instead of bld.cmd and mkrelease-haxm.cmd instead of mkrelease.cmd to build it. Looking forward to your feedback.

@leecher1337
Copy link
Contributor Author

@coxuintel Compilation process has now been simplified, maybe this facilitates testing for you?

@mirh
Copy link

mirh commented Jun 9, 2020

Needs rebase.

@leecher1337
Copy link
Contributor Author

I'll leave that as an excercise to the maintainer, as it is a lot of work (we have conflicting files now) and I have no idea when maintainer will apply the pull request (maybe it takes another year?), so it's not worth the effort.

@HaxmCI HaxmCI added CI:Build Fail CI:Build Fail and removed CI:Build Pass CI:Build Pass CI:Mac Test Pass CI:Mac Test Pass labels May 24, 2021
@wcwang wcwang force-pushed the master branch 2 times, most recently from 563eb1b to 6b942e3 Compare November 25, 2022 03:23
@wcwang wcwang force-pushed the master branch 2 times, most recently from b73a231 to da1b8ec Compare January 26, 2023 02:48
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
CI:Build Fail CI:Build Fail
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants