Re: [PATCH v4 26/30] KVM: x86: Don't treat interrupts as allowed just because a nested run is pending

[Yosry Ahmed]

> > > 2. Aside from TDX, seems like {vmx/svm}_interrupt_allowed() are doing
> > > the same thing? So maybe move all that logic into
> > > kvm_arch_interrupt_allowed(), rename it (because it's only used by
> > > x86), and make interrupt_blocked() the actual per-vendor callback?
>
> Doesn't work, at least not with more changes/hooks, because nested_exit_on_intr()
> is subtly vendor specifc.  The concept is identical, but the implementation needs
> to query vmc{b,s}12. :-/

Duh, obvious in hindsight :)

>
> I'm not opposed to figuring out a way to move the logic to common x86, because
> I agree that's where it would ideally live, but I don't want to tack that in this
> series.
>
> > > I assume this is the case because nested_run_pending used to be per-vendor,
> > > but now we can clean this up. For TDX, I assume the per-vendor hook can
> > > just be tdx_interrupt_allowed() reversed?
> >
> > It also does the renaming (nice!), so we're halfway there. I think for
> > this series, we should at least convert all direct calls to the vendor
> > .interrupt_allowed through the new kvm_is_interrupt_allowed() helper.
>
> Sadly not in this series, because handling the for_injection=true case requires
> moving the nested_run_pending logic to common x86, and without that, we end up
> with two very silly wrappers.

Yes. I think probably the way to get there is to replace the
.interrupt_allowed hook with .interrupt_blocked and a nested hook to
replace nested_exit_on_intr(), but I am not sure if performance will
matter here if we do two calls instead of one. With that, we can end
up with most of the logic in kvm_is_interrupt_allowed().

But yeah, at this point this is definitely not something to do in this series.