-
Notifications
You must be signed in to change notification settings - Fork 49
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
Add Virtualization Exception handler #629
base: main
Are you sure you want to change the base?
Conversation
TDG.VP.VMCALL<Instruction.CPUID> is used to request the VMM to emulate CPUID operations. Signed-off-by: Peter Fang <peter.fang@intel.com>
TDG.VP.VEINFO.GET is used to get Virtualization Exception Information for the recent #VE exception. Signed-off-by: Peter Fang <peter.fang@intel.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @peterfang. I will also request a review from @msft-jlange and merge upon his approval.
kernel/src/cpu/ve.rs
Outdated
let veinfo = tdcall_get_ve_info().expect("Failed to get #VE info"); | ||
|
||
match veinfo.exit_reason { | ||
VMX_EXIT_REASON_CPUID => handle_cpuid(ctx), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to consider adding IO emulation in the near future because that should be low-hanging fruit. There is much that can be copied from the instruction emulator and/or the #VC handler.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can add IO emulation after this PR. While we're on this subject, how do you feel about possibly replacing our current instruction decoder with a public crate such as zydis? I just checked and it supports no_std
+ decoder-only compilation.
The reason for this is because x86 instruction decoding is notoriously difficult to get right and bugs in this area tend to be quite challenging to triage. The decoder we previously upstreamed was an in-house development and not fully stress-tested or fuzzed. We could also continue to improve our coconut fuzzer but I'm open to exploring whether we could just delegate that to a well-known decoder. This of course would take time and would not be done right away.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not in a position to test this, but in general it looks good. I'm glad you're doing this now, because it turns out that I need to build on top of this soon. My only request is to change the file layout as I commented earlier.
Intel TD can receive #VE in certain cases and it should be handled by SVSM. Introduce a virtualization exception handler to handle them. Only the CPUID exit reason is handled right now. Co-developed-by: Peter Fang <peter.fang@intel.com> Signed-off-by: Chuanxiao Dong <chuanxiao.dong@intel.com>
Intel TD can receive #VE in certain cases and it should be handled by SVSM. Introduce a virtualization exception handler to handle them. Only the CPUID exit reason is handled right now.