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

Conversation

krytarowski
Copy link
Contributor

This module can already boot FREEDOS on NetBSD/amd64.

@krytarowski
Copy link
Contributor Author

@krytarowski
Copy link
Contributor Author

krytarowski commented Nov 26, 2018

It's not a final version, but base for further improvements, in particular NetBSD as guest still does not boot.

@raphaelning
Copy link
Contributor

Thanks! First of all, we need you to commit to the maintenance of all the NetBSD-specific code that this PR adds to the HAXM source tree. Basically, the conditions for merging #108 (see #108 (comment)) also apply to this PR.

We don't really have time to set up a NetBSD environment and test this PR. Still, if you write up some documentation (docs/manual-netbsd.md), other NetBSD users may be able to try this out.

Please allow me more time to complete the review. I'm not familiar with the BSD API, so if other people want to participate in the review (@tuxillo maybe?), please feel free to do so :-)

@tuxillo
Copy link

tuxillo commented Nov 26, 2018

BSDs are quite different at this level and I'm not really familiar with NetBSD internals. We decided not to share code initially because of this. If there are ports to other BSDs then we might decide to create some common layer but as @krytarowski pointed out, it shouldn't be the focus at this early stage.

I'll keep working on the DragonFly BSD port, and I'll submit it when it is more or less ready with the lessons learned from this PR :-)

@krytarowski
Copy link
Contributor Author

krytarowski commented Nov 26, 2018

CC: @m00nbsd - NetBSD x86 maintainer (for review of code, if there is time and interest -- for the discussion of rationale of HAXM - we already discussed internally).

These exact internals are almost completely distinct and specific to NetBSD. We are probably not more similar to other BSDs (FreeBSD, DragonFlyBSD) than to Linux or Darwin. OpenBSD is a fork of NetBSD and uses UVM (with 2 decades of divergence), but it doesn't support loadable kernel modules anyway.

  1. I'm not touching any generic code paths of other systems. All builds pass.
  2. https://github.com/NetBSD/pkgsrc-wip/tree/master/qemu-haxm contains needed qemu patches for NetBSD as host. There is a pending upstream patch for adding Linux, I will follow up with NetBSD (add a 1-liner to configure).
  3. OK. I've also marked myself a maintainer also in pkgsrc-wip/haxm (for NetBSD host).

This support isn't complete, but good enough for start. I'm using locally in particular this patch for fpudna:
http://netbsd.org/~kamil/patch-00074-permit-fpudna-in-kernel.txt I'm also using a little bit patched kernel in other parts -- not sure how relevant, but I will keep researching patches one after another - starting with this FPU handling. @m00nbsd rejected the fpudna kernel patch and requested to fix HAXM, I will find a way for it - very likely it will touch generic code paths.

I propose to reschedule docs/manual-netbsd.md until we will be able to boot into a functional shell of NetBSD on a NetBSD host. I will keep working on it and I explicitly state that this patch adds experimental support, already functional for booting FREEDOS prompt/installer.

@krytarowski
Copy link
Contributor Author

If possible I would like to drop !CONFIG_HAX_EPT2 from haxm after this commit. It will simplify code paths and need for all OS-specific APIs implemented in HAXM. In one place I'm assuming !CONFIG_HAX_EPT2 support only -- in platforms/netbsd/hax_mm.c. I've noted that Linux is also developed in mind of CONFIG_HAX_EPT2 only.

@krytarowski
Copy link
Contributor Author

As a reviewer maybe CC: @zoulasc

@raphaelning
Copy link
Contributor

If possible I would like to drop !CONFIG_HAX_EPT2 from haxm after this commit. It will simplify code paths and need for all OS-specific APIs implemented in HAXM.

Yes, that would be great. There's no need for new code to support the !CONFIG_HAX_EPT2 (legacy memory virtualization engine) code path.

@krytarowski
Copy link
Contributor Author

We don't really have time to set up a NetBSD environment and test this PR.

I recommend to revisit this statement. I'm going to prepare a command-by-command tutorial how to get it done so you (and maybe @AlexAltea ) can give it a try.

It will involve steps not applicable for future end-users (as patching the kernel).

@krytarowski
Copy link
Contributor Author

@AlexAltea
Copy link
Contributor

AlexAltea commented Nov 28, 2018

@krytarowski Thank you, I'll try to replicate your results this weekend and let you know if it works for me.

EDIT: Sorry, but the whole NetBSD setup is too cumbersome. I won't be involved in testing and maintenance of this backend.

@krytarowski
Copy link
Contributor Author

@AlexAltea thanks! I'm looking forward to it.

@krytarowski
Copy link
Contributor Author

ping?

@raphaelning
Copy link
Contributor

@krytarowski Sorry, I wasn't able to make much progress last week. I'll try to complete this review today.

@krytarowski
Copy link
Contributor Author

Useful resources:

opengrok with NetBSD sources http://src.illumos.org/source/ (http://nxr.netbsd.org/ seems to be unavailable temporarily)
man-pages http://man.netbsd.org/

There are few places for cleanups, but I'm resisting myself from mutating the pending patch prior-review and scheduling it for later.

@krytarowski
Copy link
Contributor Author

I hope a little bit for someone to give it a try and finger me what's wrong.

Copy link
Contributor

@raphaelning raphaelning left a comment

Choose a reason for hiding this comment

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

Sorry for the delayed review!

  1. I've skipped the legacy memory allocation/mapping interfaces (hax_{mem_alloc, mm}.c), since I expect them to eventually fade out.
  2. I haven't caught any serious issues. But if you decide to revise the PR based on my comments, please create new commits on the same branch, rather than amending the existing one and doing a force push. Later, you can squash the branch into a final commit.

@krytarowski
Copy link
Contributor Author

@raphaelning done.

I'm trying to leave refactoring of this code for future and get this merged as is.

@krytarowski
Copy link
Contributor Author

If the new version is acceptable, I will squash it and rebase on top of master.

Copy link
Contributor

@raphaelning raphaelning left a comment

Choose a reason for hiding this comment

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

Thanks! Please go ahead with the squash and rebase.

This module can already boot FREEDOS on NetBSD/amd64.

Signed-off-by: Kamil Rytarowski <[email protected]>
@krytarowski
Copy link
Contributor Author

Done!

@wcwang wcwang merged commit 5993f56 into intel:master Dec 10, 2018
@krytarowski
Copy link
Contributor Author

Thanks!

I've noted that it didn't take long to see HAXM in action:
https://twitter.com/sehnsuc77565442/status/1072182648125251584

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants