Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Subject: Re: [RESEND PATCH v10 06/10] vmx: spp: Set up SPP paging table at
 vmentry/vmexit
To:     Sean Christopherson <sean.j.christopherson@intel.com>,
        Yang Weijiang <weijiang.yang@intel.com>
Cc:     kvm@vger.kernel.org, linux-kernel@vger.kernel.org,
        jmattson@google.com, yu.c.zhang@linux.intel.com,
        alazar@bitdefender.com, edwin.zhai@intel.com
References: <20200102061319.10077-1-weijiang.yang@intel.com>
 <20200102061319.10077-7-weijiang.yang@intel.com>
 <20200110180458.GG21485@linux.intel.com>
 <20200113081050.GF12253@local-michael-cet-test.sh.intel.com>
 <20200113173358.GC1175@linux.intel.com>
 <20200114030820.GA4583@local-michael-cet-test.sh.intel.com>
 <20200114185808.GI16784@linux.intel.com>
From:   Paolo Bonzini <pbonzini@redhat.com>
Message-ID: <5d4118d9-c501-6c59-82cc-50e317be195c@redhat.com>
Date:   Tue, 21 Jan 2020 15:14:37 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.1.1
MIME-Version: 1.0
In-Reply-To: <20200114185808.GI16784@linux.intel.com>
Content-Type: text/plain; charset=windows-1252
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On 14/01/20 19:58, Sean Christopherson wrote:
> I'm not convinced the instruction length needs to be provided to userspace
> for this case.  Obviously it's not difficult to provide the info, I just
> don't understand the value added by doing so.  As above, RIP shouldn't
> need to be unwound, and blindly skipping an instruction seems like an odd
> thing for a VMI engine to do.
> 

The reason to introduce the instruction length was so that userspace
could blindly use SPP to emulate ROM behavior.  Weijiang's earlier
patches in fact _only_ provided that behavior, and I asked him to change
it so that, by default, using SPP and not handling it will basically
cause an infinite loop of SPP violations.

One possibility to clean things up is to change "fault_handled" and
fast_page_fault's return value from bool to RET_PF* (false becomes
RET_PF_INVALID, true becomes RET_PF_RETRY).  direct_page_fault would
also have to do something like

	r = fast_page_fault(vcpu, gpa, level, error_code))
	if (r != RET_PF_INVALID)
		return r;

Then fast_page_fault can just return RET_PF_USERSPACE, and this ugly
case can go away.

+		if (vcpu->run->exit_reason == KVM_EXIT_SPP)
+			r = RET_PF_USERSPACE;
+

Thanks,

Paolo