On Wed, Feb 15, 2023, Anish Moorthy wrote:
> With the relevant kvm cap enabled, EPT violations will exit to userspace
> w/ reason KVM_EXIT_MEMORY_FAULT instead of resolving the fault via slow
> get_user_pages.
Similar to the other patch's feedback, please rewrite the changelog to phrase the
changes as commands, not as descriptions of the resulting behavior.
> Signed-off-by: Anish Moorthy <amoorthy@xxxxxxxxxx>
> Suggested-by: James Houghton <jthoughton@xxxxxxxxxx>
> ---
> arch/x86/kvm/mmu/mmu.c | 23 ++++++++++++++++++++---
> arch/x86/kvm/x86.c | 1 +
> 2 files changed, 21 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index aeb240b339f54..28af8d60adee6 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4201,6 +4201,7 @@ static int __kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> {
> struct kvm_memory_slot *slot = fault->slot;
> bool async;
> + bool mem_fault_nowait;
>
> /*
> * Retry the page fault if the gfn hit a memslot that is being deleted
> @@ -4230,9 +4231,25 @@ static int __kvm_faultin_pfn(struct kvm_vcpu *vcpu, struct kvm_page_fault *fault
> }
>
> async = false;
> - fault->pfn = __gfn_to_pfn_memslot(slot, fault->gfn, false, false, &async,
> - fault->write, &fault->map_writable,
> - &fault->hva);
> + mem_fault_nowait = memory_faults_enabled(vcpu->kvm);
> +
> + fault->pfn = __gfn_to_pfn_memslot(
> + slot, fault->gfn,
> + mem_fault_nowait,
Hrm, as prep work for this series, I think we should first clean up the pile o'
bools. This came up before when the "interruptible" flag was added[*]. We punted
then, but I think it's time to bite the bullet, especially since "nowait" and
"async" need to be mutually exclusive.
[*] https://lore.kernel.org/all/YrR9i3yHzh5ftOxB@xxxxxxxxxx
> + false,
> + mem_fault_nowait ? NULL : &async,
> + fault->write, &fault->map_writable,
> + &fault->hva);
Same comments as the other patch: follow kernel style, not google3's madness :-)
> + if (mem_fault_nowait) {
> + if (fault->pfn == KVM_PFN_ERR_FAULT) {
> + vcpu->run->exit_reason = KVM_EXIT_MEMORY_FAULT;
> + vcpu->run->memory_fault.gpa = fault->gfn << PAGE_SHIFT;
> + vcpu->run->memory_fault.size = PAGE_SIZE;
This belongs in a separate patch, and the exit stuff should be filled in by
kvm_handle_error_pfn(). Then this if-statement goes away entirely because the
"if (!async)" will always evaluate true in the nowait case.
> + }
> + return RET_PF_CONTINUE;
> + }
> +
> if (!async)
> return RET_PF_CONTINUE; /* *pfn has correct page already */
>
