Re: [PATCH 3/7] KVM: TDX: Retry locally in TDX EPT violation handler on RET_PF_RETRY

[Sean Christopherson <seanjc@google.com>]

On Mon, Jan 27, 2025, Yan Zhao wrote:
> On Fri, Jan 24, 2025 at 05:23:36PM -0800, Sean Christopherson wrote:
> My previous consideration is that
> when there's a pending interrupt that can be recognized, given the current VM
> Exit reason is EPT violation, the next VM Entry will not deliver the interrupt
> since the condition to recognize and deliver interrupt is unchanged after the
> EPT violation VM Exit.
> So checking pending interrupt brings only false positive, which is unlike
> checking PID that the vector in the PID could arrive after the EPT violation VM
> Exit and PID would be cleared after VM Entry even if the interrupts are not
> deliverable. So checking PID may lead to true positive and less false positive.
> 
> But I understand your point now. As checking PID can also be false positive, it
> would be no harm to introduce another source of false positive.

Yep.  FWIW, I agree that checking VMXIP is theoretically "worse", in the sense
that it's much more likely to be a false positive.  Off the top of my head, the
only time VMXIP will be set with RFLAGS.IF=1 on an EPT Violation is if the EPT
violation happens in an STI or MOV_SS/POP_SS shadow.

> So using kvm_vcpu_has_events() looks like a kind of trade-off?
> kvm_vcpu_has_events() can make TDX's code less special but might lead to the
> local vCPU more vulnerable to the 0-step mitigation, especially when interrupts
> are disabled in the guest.

Ya.  I think it's worth worth using kvm_vcpu_has_events() though.  In practice,
observing VMXIP=1 with RFLAGS=0 on an EPT Violation means the guest is accessing
memory for the first time in atomic kernel context.  That alone seems highly
unlikely.  Add in that triggering retry requires an uncommon race, and the overall
probability becomes miniscule.

> > That code needs a comment, because depending on the behavior of that field, it
> > might not even be correct.
> > 
> > > (2) kvm_vcpu_has_events() may lead to unnecessary breaks due to exception
> > >     pending. However, vt_inject_exception() is NULL for TDs.
> > 
> > Wouldn't a pending exception be a KVM bug?
> Hmm, yes, it should be.
> Although kvm_vcpu_ioctl_x86_set_mce() can invoke kvm_queue_exception() to queue
> an exception for TDs, 

I thought TDX didn't support synthetic #MCs?

> this should not occur while VCPU_RUN is in progress.

Not "should not", _cannot_, because they're mutually exclusive.