genodelabs / genode

Genode OS Framework
https://genode.org/
Other
1.07k stars 254 forks source link

hw: implement nested page table #5218

Closed atopia closed 5 months ago

atopia commented 5 months ago

Implement a nested page table for x86 virtualization using the x86 page table template base implementation.

atopia commented 5 months ago

I've successfully tested this branch with Sculpt and @jschlatow's firefox-av VM package albeit somehow audio wasn't playing, but we're still talking about a Linux VM here :smiley: No seriously, I still have to check that audio works in principle but from my quick test, it seems that the current state can be offered as "First Sculpt PC variant on the base-hw kernel" with the firefox-av VM as test case.

chelmuth commented 5 months ago

That's great news, however I have follow-up questions.

I must admit I'm unsure about the state of the branch. Thus, it would be good if you notify me if it's ready for merge and @jschlatow as well as @skalk approve. Setting the [fixed] label would be sensible, then.

atopia commented 5 months ago
* Looking at the patch only, it looks like both - EPT and NPT - are handled by the same page-table implementation, right?

Correct, but unfortunately this is incorrect behavior. The standard x86 page table type was correct for use with SVM, and the new EPT implementation should be correct for VMX. Since I mindlessly just replaced the Vm_page_table type, this means that nested paging now doesn't work correctly on SVM anymore. I'll try to come up with a good way of switching implementations at run time.

atopia commented 5 months ago
* If _clflush.h_ must be moved from _drivers/platform/pc_ to _base/include_, it should be placed in the  _spec/x86/cpu/_ subdirectory.

If fixed this with a fixup commit in #5217.

atopia commented 5 months ago

I uploaded two versions that pick the nested page table implementation at runtime. The first version (b8dd28e) works without changes to the surrounding code and without extra data members safe for a static enum to pic the virtualization type, at the expense of being not so nice C++. If tried to use a virtual base class and a lambda in the second commit on a separate branch (fe899c7), but I wasn't able to get it to work yet, and it incurs changes to the x86-specific implementation of the VM Session Component.

atopia commented 5 months ago

Maybe @nfeske has an idea how to get to a better working version?

nfeske commented 5 months ago

Maybe @nfeske has an idea how to get to a better working version?

I think the first version is fine for now. I agree that your second commit goes in a good direction. But in my opinion we should not force it in the short term of the current release. Let us keep the clear separation of both page table formats for a subsequent step (without haste) after the release.

atopia commented 5 months ago

OK. I pushed the first version of my Vm_page_table runtime selection code with an updated commit message. I would suggest to merge this version on top of the EPT implementation commit, as the suboptimal part

If this implementation is acceptable for now, I will open an issue to clean up the Vm_page_table implementation and provide cached virtualization technology detection at a higher level.

chelmuth commented 5 months ago

@atopia please check current staging for missing commits.

atopia commented 5 months ago

Staging looks good to me, thanks!