openhcl/tdx: inject machine check on ept exits caused by bad guest addresses #1340
Conversation
| // For now, we just check if the exit was suprious or if we | ||
| // should inject a machine check. An exit is considered spurious | ||
| // if the gpa is accessible. | ||
| if self.partition.gm[intercepted_vtl].check_gpa_readable(gpa) { |
There was a problem hiding this comment.
Do we want to copy these checks over to SNP? Or put them in some shared location in hardware_cvm?
There was a problem hiding this comment.
I think so yes - let me see if i can do a quick attempt. But SNP also needs to be fixed to not rely on the hypervisor supplied intercept message.
There was a problem hiding this comment.
fixing SNP requires digging thru the specs. I'd like to take the TDX change on the first try so we can get some wider coverage.
| ?ept_info, | ||
| "guest accessed gpa not described in memory layout, injecting MC" | ||
| ); | ||
| self.inject_mc(intercepted_vtl); |
There was a problem hiding this comment.
I worry this is too aggressive--it's not necessarily the case that all MMIO resides inside the MMIO ranges that are listed in the topology. E.g., I don't see how XAPIC works with this approach.
So I think either you need to weaken this check, or we need to do more work to ensure the topology is completely accurate.
There was a problem hiding this comment.
hmm... we don't support XAPIC today in TDX today, right, only x2apic?
I think you are right though that this check is very strict. Note that this is what our other paravisor implementation does.
For non CVMs I am a bit worried about this being overzealous (and we probably don't need to be so strict). I bet we'd need to make this handling a bit more configurable - let me think about it some more.
| ) | ||
| .await?; | ||
| } | ||
| Some(AddressType::Ram) => { |
There was a problem hiding this comment.
We need to handle overlay pages as well. Specifically, the MNF overlay will be in RAM but needs to be handled via emulation.
There was a problem hiding this comment.
we don't support MNF today right? the intention about this being limited was the TODO comment above about VTL pages and MNF
| .lower_vtl_memory_layout | ||
| .probe_address(canonical_gpa); | ||
|
|
||
| match address_type { |
There was a problem hiding this comment.
Probably this logic should ultimately move into the virt_support_x86emu crate so that we don't have to replicate it everywhere. I.e., we should probably do this same thing for standard VMs with openvmm, right?
There was a problem hiding this comment.
Agreed. We discussed a bit offline, but I think we'll need to make the policy configurable. I'll take a stab.
chris-oo
force-pushed
the
inject-mc-tdx
branch
2 times, most recently
from
925a86b to
57c5608
Compare
chris-oo
added
the
backport_2505
The guest should never touch addresses that are not described to it in the memory layout. Previously, we emulated this exit but we should instead be injecting a machine check.
Additionally, a guest accessing a page with the incorrect shared bit set also results in a machine check. This otherwise will result in an emulation failure later, causing an OpenHCL crash.
PENDING: tests