Messages in this thread | ![/](/images/icornerl.gif) | | From | "Edgecombe, Rick P" <> | Subject | Re: [RFC PATCH 05/13] kvm: Add #PF injection for KVM XO | Date | Fri, 4 Oct 2019 19:11:16 +0000 |
| |
On Fri, 2019-10-04 at 09:42 +0200, Paolo Bonzini wrote: > On 03/10/19 23:23, Rick Edgecombe wrote: > > + if (!vcpu->arch.gva_available) > > + return 0; > > Please return RET_PF_* constants, RET_PF_EMULATE here.
Ok.
> > + if (error_code & PFERR_WRITE_MASK) > > + fault_error_code |= X86_PF_WRITE; > > + > > + fault.vector = PF_VECTOR; > > + fault.error_code_valid = true; > > + fault.error_code = fault_error_code; > > + fault.nested_page_fault = false; > > + fault.address = vcpu->arch.gva_val; > > + fault.async_page_fault = true; > > Not an async page fault.
Right.
> > + kvm_inject_page_fault(vcpu, &fault); > > + > > + return 1; > > Here you would return RET_PF_RETRY - you've injected the page fault and > all that's left to do is reenter execution of the vCPU. > > [...] > > > + if (unlikely(vcpu->arch.xo_fault)) { > > + /* > > + * If not enough information to inject the fault, > > + * emulate to figure it out and emulate the PF. > > + */ > > + if (!try_inject_exec_only_pf(vcpu, error_code)) > > + return RET_PF_EMULATE; > > + > > + return 1; > > + } > > Returning 1 is wrong, it's also RET_PF_EMULATE. If you change > try_inject_exec_only_pf return values to RET_PF_*, you can simply return > the value of try_inject_exec_only_pf(vcpu, error_code).
Oh right! I must have broken this at some point. Thanks.
> That said, I wonder if it's better to just handle this in > handle_ept_violation. Basically, if bits 5:3 of the exit qualification > are 100 you can bypass the whole mmu.c page fault handling and just > inject an exec-only page fault. > > Thanks, > > Paolo
Hmm, that could be cleaner. I'll see how it fits together when I fix the nested case, since some of that logic looks to be in mmu.c.
Thanks,
Rick
| ![\](/images/icornerr.gif) |