Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: qLCGo+iyW5O1JSOnOSsv1dq/jtaV76EGpwu1yhLYo6KzJrHBQs6s73tcFkK7OSWlpGwHSIkg78
 m/Mp3p4AXDUw==
IronPort-SDR: vuidcrLsSHUvLnHM78ClpxSJeqGc1d3MaeNx3OBTZpmjtVUy5Y5t1NWeE+1lmsl8lGzaGx7iSI
 EjIelMGv2OzA==
Date:   Fri, 5 Jun 2020 13:06:51 -0700
From:   Sean Christopherson <sean.j.christopherson@intel.com>
To:     Vitaly Kuznetsov <vkuznets@redhat.com>
Cc:     kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>,
        Wanpeng Li <wanpengli@tencent.com>,
        Jim Mattson <jmattson@google.com>, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/2] KVM: nVMX: Properly handle
 kvm_read/write_guest_virt*() result
Message-ID: <20200605200651.GC11449@linux.intel.com>
References: <20200605115906.532682-1-vkuznets@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20200605115906.532682-1-vkuznets@redhat.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Fri, Jun 05, 2020 at 01:59:05PM +0200, Vitaly Kuznetsov wrote:
> Introduce vmx_handle_memory_failure() as an interim solution.

Heh, "interim".  I'll take the over on that :-D.

> Note, nested_vmx_get_vmptr() now has three possible outcomes: OK, PF,
> KVM_EXIT_INTERNAL_ERROR and callers need to know if userspace exit is
> needed (for KVM_EXIT_INTERNAL_ERROR) in case of failure. We don't seem
> to have a good enum describing this tristate, just add "int *ret" to
> nested_vmx_get_vmptr() interface to pass the information.
> 
> Reported-by: syzbot+2a7156e11dc199bdbd8a@syzkaller.appspotmail.com
> Suggested-by: Sean Christopherson <sean.j.christopherson@intel.com>
> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com>
> ---

...

> +/*
> + * Handles kvm_read/write_guest_virt*() result and either injects #PF or returns
> + * KVM_EXIT_INTERNAL_ERROR for cases not currently handled by KVM. Return value
> + * indicates whether exit to userspace is needed.
> + */
> +int vmx_handle_memory_failure(struct kvm_vcpu *vcpu, int r,
> +			      struct x86_exception *e)
> +{
> +	if (r == X86EMUL_PROPAGATE_FAULT) {
> +		kvm_inject_emulated_page_fault(vcpu, e);
> +		return 1;
> +	}
> +
> +	/*
> +	 * In case kvm_read/write_guest_virt*() failed with X86EMUL_IO_NEEDED
> +	 * while handling a VMX instruction KVM could've handled the request

A nit similar to your observation on the shortlog, this isn't limited to VMX
instructions.

> +	 * correctly by exiting to userspace and performing I/O but there
> +	 * doesn't seem to be a real use-case behind such requests, just return
> +	 * KVM_EXIT_INTERNAL_ERROR for now.
> +	 */
> +	vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
> +	vcpu->run->internal.suberror = KVM_INTERNAL_ERROR_EMULATION;
> +	vcpu->run->internal.ndata = 0;
> +
> +	return 0;
> +}