| Message ID | 20240809190319.1710470-1-seanjc@google.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | KVM: x86: Fix multiple #PF RO infinite loop bugs | expand |
On 8/9/24 21:02, Sean Christopherson wrote: > The folks doing TDX enabling ran into a problem where exposing a read-only > memslot to a TDX guest put it into an infinite loop. The most immediate > issue is that KVM never creates MMIO SPTEs for RO memslots, because except > for TDX (which isn't officially supported yet), such SPTEs can't distinguish > between reads and writes, i.e. would trigger MMIO on everything and thus > defeat the purpose of having a RX memslot. > > That breaks TDX, SEV-ES, and SNP, i.e. VM types that rely on MMIO caching > to reflect MMIO faults into the guest as #VC/#VE, as the guest never sees > the fault, KVM refuses to emulate, the guest loops indefinitely. That's > patch 1. > > Patches 2-4 fix an amusing number of other bugs that made it difficult to > figure out the true root cause. > > The rest is a bunch of cleanups to consolidate all of the unprotect+retry > paths (there are four-ish). > > As a bonus, adding RET_PF_WRITE_PROTECTED obviates the need for > kvm_lookup_pfn()[*]. > > [*] https://lore.kernel.org/all/63c41e25-2523-4397-96b4-557394281443@redhat.com Nice! For now I've placed it in kvm/queue as this is clearly 6.12 material. It will be replaced by the v2 of course before graduating to kvm/next. Thanks, Paolo > Sean Christopherson (22): > KVM: x86: Disallow read-only memslots for SEV-ES and SEV-SNP (and TDX) > KVM: VMX: Set PFERR_GUEST_{FINAL,PAGE}_MASK if and only if the GVA is > valid > KVM: x86/mmu: Trigger unprotect logic only on write-protection page > faults > KVM: x86/mmu: Skip emulation on page fault iff 1+ SPs were unprotected > KVM: x86: Retry to-be-emulated insn in "slow" unprotect path iff sp is > zapped > KVM: x86: Get RIP from vCPU state when storing it to last_retry_eip > KVM: x86: Store gpa as gpa_t, not unsigned long, when unprotecting for > retry > KVM: x86/mmu: Apply retry protection to "fast nTDP unprotect" path > KVM: x86/mmu: Try "unprotect for retry" iff there are indirect SPs > KVM: x86/mmu: Replace PFERR_NESTED_GUEST_PAGE with a more descriptive > helper > KVM: x86: Move EMULTYPE_ALLOW_RETRY_PF to x86_emulate_instruction() > KVM: x86: Fold retry_instruction() into x86_emulate_instruction() > KVM: x86/mmu: Don't try to unprotect an INVALID_GPA > KVM: x86/mmu: Always walk guest PTEs with WRITE access when > unprotecting > KVM: x86/mmu: Move event re-injection unprotect+retry into common path > KVM: x86: Remove manual pfn lookup when retrying #PF after failed > emulation > KVM: x86: Check EMULTYPE_WRITE_PF_TO_SP before unprotecting gfn > KVM: x86: Apply retry protection to "unprotect on failure" path > KVM: x86: Update retry protection fields when forcing retry on > emulation failure > KVM: x86: Rename > reexecute_instruction()=>kvm_unprotect_and_retry_on_failure() > KVM: x86/mmu: Subsume kvm_mmu_unprotect_page() into the and_retry() > version > KVM: x86/mmu: Detect if unprotect will do anything based on > invalid_list > > arch/x86/include/asm/kvm_host.h | 16 ++- > arch/x86/kvm/mmu/mmu.c | 175 ++++++++++++++++++++++---------- > arch/x86/kvm/mmu/mmu_internal.h | 3 + > arch/x86/kvm/mmu/mmutrace.h | 1 + > arch/x86/kvm/mmu/paging_tmpl.h | 2 +- > arch/x86/kvm/mmu/tdp_mmu.c | 6 +- > arch/x86/kvm/vmx/vmx.c | 5 +- > arch/x86/kvm/x86.c | 133 +++++++----------------- > include/linux/kvm_host.h | 7 ++ > virt/kvm/kvm_main.c | 5 +- > 10 files changed, 184 insertions(+), 169 deletions(-) > > > base-commit: 332d2c1d713e232e163386c35a3ba0c1b90df83f
The folks doing TDX enabling ran into a problem where exposing a read-only memslot to a TDX guest put it into an infinite loop. The most immediate issue is that KVM never creates MMIO SPTEs for RO memslots, because except for TDX (which isn't officially supported yet), such SPTEs can't distinguish between reads and writes, i.e. would trigger MMIO on everything and thus defeat the purpose of having a RX memslot. That breaks TDX, SEV-ES, and SNP, i.e. VM types that rely on MMIO caching to reflect MMIO faults into the guest as #VC/#VE, as the guest never sees the fault, KVM refuses to emulate, the guest loops indefinitely. That's patch 1. Patches 2-4 fix an amusing number of other bugs that made it difficult to figure out the true root cause. The rest is a bunch of cleanups to consolidate all of the unprotect+retry paths (there are four-ish). As a bonus, adding RET_PF_WRITE_PROTECTED obviates the need for kvm_lookup_pfn()[*]. [*] https://lore.kernel.org/all/63c41e25-2523-4397-96b4-557394281443@redhat.com Sean Christopherson (22): KVM: x86: Disallow read-only memslots for SEV-ES and SEV-SNP (and TDX) KVM: VMX: Set PFERR_GUEST_{FINAL,PAGE}_MASK if and only if the GVA is valid KVM: x86/mmu: Trigger unprotect logic only on write-protection page faults KVM: x86/mmu: Skip emulation on page fault iff 1+ SPs were unprotected KVM: x86: Retry to-be-emulated insn in "slow" unprotect path iff sp is zapped KVM: x86: Get RIP from vCPU state when storing it to last_retry_eip KVM: x86: Store gpa as gpa_t, not unsigned long, when unprotecting for retry KVM: x86/mmu: Apply retry protection to "fast nTDP unprotect" path KVM: x86/mmu: Try "unprotect for retry" iff there are indirect SPs KVM: x86/mmu: Replace PFERR_NESTED_GUEST_PAGE with a more descriptive helper KVM: x86: Move EMULTYPE_ALLOW_RETRY_PF to x86_emulate_instruction() KVM: x86: Fold retry_instruction() into x86_emulate_instruction() KVM: x86/mmu: Don't try to unprotect an INVALID_GPA KVM: x86/mmu: Always walk guest PTEs with WRITE access when unprotecting KVM: x86/mmu: Move event re-injection unprotect+retry into common path KVM: x86: Remove manual pfn lookup when retrying #PF after failed emulation KVM: x86: Check EMULTYPE_WRITE_PF_TO_SP before unprotecting gfn KVM: x86: Apply retry protection to "unprotect on failure" path KVM: x86: Update retry protection fields when forcing retry on emulation failure KVM: x86: Rename reexecute_instruction()=>kvm_unprotect_and_retry_on_failure() KVM: x86/mmu: Subsume kvm_mmu_unprotect_page() into the and_retry() version KVM: x86/mmu: Detect if unprotect will do anything based on invalid_list arch/x86/include/asm/kvm_host.h | 16 ++- arch/x86/kvm/mmu/mmu.c | 175 ++++++++++++++++++++++---------- arch/x86/kvm/mmu/mmu_internal.h | 3 + arch/x86/kvm/mmu/mmutrace.h | 1 + arch/x86/kvm/mmu/paging_tmpl.h | 2 +- arch/x86/kvm/mmu/tdp_mmu.c | 6 +- arch/x86/kvm/vmx/vmx.c | 5 +- arch/x86/kvm/x86.c | 133 +++++++----------------- include/linux/kvm_host.h | 7 ++ virt/kvm/kvm_main.c | 5 +- 10 files changed, 184 insertions(+), 169 deletions(-) base-commit: 332d2c1d713e232e163386c35a3ba0c1b90df83f