On Wed, Dec 11, 2024, Ivan Orlov wrote:
> On 12/11/24 18:15, Sean Christopherson wrote:
> > 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);
> > }
> >
>
> Hm, by the way, what is the desired behaviour if EMULTYPE_ALLOW_RETRY_PF is
> set? Is it correct that we return an internal error if it is set during
> vectoring? Or KVM may try to unprotect the page and re-execute?
Heh, it's sneaky, but EMULTYPE_ALLOW_RETRY_PF can be set if and only if
RET_PF_WRITE_PROTECTED is set. Hmm, that makes me think we should do the below
(EMULTYPE_WRITE_PF_TO_SP was a recent addition).
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 2e713480933a..de5f6985d123 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9077,7 +9077,7 @@ int x86_emulate_instruction(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
if ((emulation_type & EMULTYPE_ALLOW_RETRY_PF) &&
(WARN_ON_ONCE(is_guest_mode(vcpu)) ||
- WARN_ON_ONCE(!(emulation_type & EMULTYPE_PF))))
+ WARN_ON_ONCE(!(emulation_type & EMULTYPE_WRITE_PF_TO_SP))))
emulation_type &= ~EMULTYPE_ALLOW_RETRY_PF;
r = kvm_check_emulate_insn(vcpu, emulation_type, insn, insn_len);
That said, let me get back to you on this when my brain is less tired. I'm not
sure emulating when an exit occurred during event delivery is _ever_ correct.
> If so, we may need something like
>
> static inline bool kvm_can_emulate_event_vectoring(int emul_type)
> {
> return !(emul_type & EMULTYPE_PF) ||
> (emul_type & ~(EMULTYPE_PF));
> }
>
> So it returns true if EMULTYPE_PF is not set or if it's not the only set
> bit.
