On Mon, Nov 11, 2024, Ivan Orlov wrote:
> Move unhandleable vmexit due to MMIO during vectoring error detection
> into check_emulate_instruction. Implement a function which checks if
> emul_type indicates MMIO so it can be used for both VMX and SVM.
>
> Fix the comment about EMULTYPE_PF as this flag doesn't necessarily
> mean MMIO anymore: it can also be set due to the write protection
> violation.
>
> Signed-off-by: Ivan Orlov <iorlov@xxxxxxxxxx>
> ---
> V1 -> V2:
> - Detect the unhandleable vectoring error in vmx_check_emulate_instruction
> instead of handling it in the common MMU code (which is specific for
> cached MMIO)
>
> arch/x86/include/asm/kvm_host.h | 10 ++++++++--
> arch/x86/kvm/vmx/vmx.c | 25 ++++++++++++-------------
> 2 files changed, 20 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index eb413079b7c6..3de9702a9135 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -2017,8 +2017,8 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
> * VMware backdoor emulation handles select instructions
> * and reinjects the #GP for all other cases.
> *
> - * EMULTYPE_PF - Set when emulating MMIO by way of an intercepted #PF, in which
> - * case the CR2/GPA value pass on the stack is valid.
> + * EMULTYPE_PF - Set when an intercepted #PF triggers the emulation, in which case
> + * the CR2/GPA value pass on the stack is valid.
> *
> * EMULTYPE_COMPLETE_USER_EXIT - Set when the emulator should update interruptibility
> * state and inject single-step #DBs after skipping
> @@ -2053,6 +2053,12 @@ u64 vcpu_tsc_khz(struct kvm_vcpu *vcpu);
> #define EMULTYPE_COMPLETE_USER_EXIT (1 << 7)
> #define EMULTYPE_WRITE_PF_TO_SP (1 << 8)
>
> +static inline bool kvm_is_emul_type_mmio(int emul_type)
Hmm, this should probably be "pf_mmio", not just "mmio". E.g. if KVM is emulating
large swaths of guest code because unrestricted guest is disabled, then can end up
emulating an MMIO access for "normal" emulation.
Hmm, actually, what if we go with this?
static inline bool kvm_can_emulate_event_vectoring(int emul_type)
{
return !(emul_type & EMULTYPE_PF) ||
(emul_type & EMULTYPE_WRITE_PF_TO_SP);
}
The MMIO aspect isn't unique to VMX or SVM, and the above would allow handling
other incompatible emulation types, should they ever arise (unlikely).
More importantly, it provides a single location to document (via comment) exactly
why KVM can't deal with MMIO emulation during event vectoring (and why KVM allows
emulation of write-protect page faults).
It would require using X86EMUL_UNHANDLEABLE_VECTORING instead of
X86EMUL_UNHANDLEABLE_VECTORING_IO, but I don't think that's necessarily a bad
thing.
> +{
> + return (emul_type & EMULTYPE_PF) &&
> + !(emul_type & EMULTYPE_WRITE_PF_TO_SP);
> +}
> +
> int kvm_emulate_instruction(struct kvm_vcpu *vcpu, int emulation_type);
> int kvm_emulate_instruction_from_buffer(struct kvm_vcpu *vcpu,
> void *insn, int insn_len);
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index f92740e7e107..a10f35d9704b 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -1693,6 +1693,8 @@ static int vmx_rtit_ctl_check(struct kvm_vcpu *vcpu, u64 data)
> int vmx_check_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
> void *insn, int insn_len)
> {
> + bool is_vect;
> +
> /*
> * Emulation of instructions in SGX enclaves is impossible as RIP does
> * not point at the failing instruction, and even if it did, the code
> @@ -1704,6 +1706,13 @@ int vmx_check_emulate_instruction(struct kvm_vcpu *vcpu, int emul_type,
> kvm_queue_exception(vcpu, UD_VECTOR);
> return X86EMUL_PROPAGATE_FAULT;
> }
> +
> + is_vect = to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK;
> +
> + /* Emulation is not possible when MMIO happens during event vectoring. */
> + if (kvm_is_emul_type_mmio(emul_type) && is_vect)
I definitely prefer to omit the local variable.
if ((to_vmx(vcpu)->idt_vectoring_info & VECTORING_INFO_VALID_MASK) &&
!kvm_can_emulate_event_vectoring(emul_type))
return X86EMUL_UNHANDLEABLE_VECTORING;
