Re: [PATCH] x86/HVM: don't mark evtchn upcall vector as pending when vLAPIC is disabled

["Roger Pau Monné" <roger.pau@citrix.com>]

On Fri, Nov 18, 2022 at 12:33:00PM +0000, Andrew Cooper wrote:
> On 18/11/2022 10:31, Jan Beulich wrote:
> > Linux'es relatively new use of HVMOP_set_evtchn_upcall_vector has
> > exposed a problem with the marking of the respective vector as
> > pending: For quite some time Linux has been checking whether any stale
> > ISR or IRR bits would still be set while preparing the LAPIC for use.
> > This check is now triggering on the upcall vector, as the registration,
> > at least for APs, happens before the LAPIC is actually enabled.
> >
> > In software-disabled state an LAPIC would not accept any interrupt
> > requests and hence no IRR bit would newly become set while in this
> > state. As a result it is also wrong for us to mark the upcall vector as
> > having a pending request when the vLAPIC is in this state.
> 
> I agree with this.
> 
> > To compensate for the "enabled" check added to the assertion logic, add
> > logic to (conditionally) mark the upcall vector as having a request
> > pending at the time the LAPIC is being software-enabled by the guest.
> 
> But this, I don't think is appropriate.
> 
> The point of raising on enable is allegedly to work around setup race
> conditions.  I'm unconvinced by this reasoning, but it is what it is,
> and the stated behaviour is to raise there and then.
> 
> If a guest enables evtchn while the LAPIC is disabled, then the
> interrupt is lost.  Like every other interrupt in an x86 system.
> 
> I don't think there is any credible way a guest kernel author can expect
> the weird evtchn edgecase to wait for an arbitrary point in the future,
> and it's a corner case that I think is worth not keeping.

We would then need some kind of fix in order to clear
evtchn_upcall_pending, because having that set without an interrupt
pending on the vLAPIC will result in no further event channel callback
interrupts being injected (see vcpu_mark_events_pending()).

Maybe we want to change vcpu_mark_events_pending() so that it always
tries to inject the vector even if evtchn_upcall_pending is already
set by calling hvm_assert_evtchn_irq() unconditionally?

Thanks, Roger.