On Mon, May 06, 2024, Chao Gao wrote:
> On Wed, May 01, 2024 at 09:54:07AM -0700, Reinette Chatre wrote:
> >diff --git a/arch/x86/kvm/vmx/tdx.c b/arch/x86/kvm/vmx/tdx.c
> >index 499c6cd9633f..ba81e6f68c97 100644
> >--- a/arch/x86/kvm/vmx/tdx.c
> >+++ b/arch/x86/kvm/vmx/tdx.c
> >@@ -1305,11 +1305,20 @@ static int tdx_handle_ept_violation(struct kvm_vcpu *vcpu)
> > } else {
> > exit_qual = tdexit_exit_qual(vcpu);
> > if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
> >+ union tdx_exit_reason exit_reason = to_tdx(vcpu)->exit_reason;
> >+
> >+ /*
> >+ * Instruction fetch in TD from shared memory
> >+ * causes a #PF.
> >+ */
>
> It is better to hoist this comment above the if-statement.
>
> /*
> * Instruction fetch in TD from shared memory never causes EPT
> * violation. Warn if such an EPT violation occurs as the CPU
> * probably is buggy.
> */
> if (exit_qual & EPT_VIOLATION_ACC_INSTR) {
> ...
> }
>
>
> but, I am wondering why KVM needs to perform this check. I prefer to drop this
> check if KVM doesn't rely on it to handle EPT violation correctly.
>
> > pr_warn("kvm: TDX instr fetch to shared GPA = 0x%lx @ RIP = 0x%lx\n",
> > tdexit_gpa(vcpu), kvm_rip_read(vcpu));
>
> how about using vcpu_unimpl()? it is a wrapper for printing such a message.
This isn't an "unimplemented" scenario in KVM, this is a "hardware is broken"
scenario. Just WARN_ON_ONCE() and move on. Or I suppose since EPT misconfig
needs to do something as well:
if (KVM_BUG_ON(exit_qual & EPT_VIOLATION_ACC_INSTR, vcpu->kvm))
return -EIO;
