On 24.11.2022 10:33, Roger Pau Monné wrote: > On Thu, Nov 24, 2022 at 10:11:05AM +0100, Jan Beulich wrote: >> On 24.11.2022 10:06, Roger Pau Monné wrote: >>> On Thu, Nov 24, 2022 at 09:42:40AM +0100, Roger Pau Monné wrote: >>>> On Thu, Nov 24, 2022 at 08:59:00AM +0100, Jan Beulich wrote: >>>>> - problematic wrt evtchn_upcall_pending, once set, preventing event >>>>> injection later on. >>>>> As you may have inferred already, I'm inclined to suggest to drop the >>>>> the is_vcpu_online() check from hvm_set_callback_via(). >>>>> >>>>> One related question here is whether vlapic_do_init() shouldn't have >>>>> the non-architectural side effect of clearing evtchn_upcall_pending. >>>>> While this again violates the principle of the hypervisor only ever >>>>> setting that bit, it would deal with the risk of no further event >>>>> injection once the flag is set, considering that vlapic_do_init() >>>>> clears IRR (and ISR). >>>> >>>> That would seem sensible to me, and was kind of what I was suggesting >>>> in: >>>> >>>> https://lore.kernel.org/xen-devel/Y3eO0bMKRPYJc2yQ@Air-de-Roger/ >>> >>> Another option would be for vcpu_mark_events_pending() to >>> unconditionally call hvm_assert_evtchn_irq() regardless of the state >>> of evtchn_upcall_pending. >> >> I think you said so before, and ... >> >>> This will create some spurious events. >> >> ... I continue to be afraid of s/some/many/. > > Not _that_ many I think, as we can only queue one pending interrupt in > IRR.
We need to be careful here - the kernel treating it as "edge" (like any other interrupt coming directly from the LAPIC), it ack-s it before calling the handler, i.e. before evtchn_upcall_pending would have a chance to be cleared. See Linux'es sysvec_xen_hvm_callback(). >> This event delivery is meant >> to be kind of edge triggered, and I think it is a reasonable model to treat >> the flag going from 0 to 1 as the "edge". > > Hm, it's a weird interrupt model I would say. In some aspects it's > similar to level (as the line stays asserted until it's cleared), but > we don't get new interrupts injected into the APIC. > > Maybe the right mode would be to treat evtchn_upcall_pending as a > level triggered line and keep injecting interrupts until the line is > deasserted (ie: evtchn_upcall_pending == 0)? As said above, and as also pointed out by Andrew on another sub- thread when discussing the (dis)similarity with IO-APIC connected interrupts, at the LAPIC there's no edge/level distinction anymore, as we're not dealing with "asserted" signals there, but just with messages sent on the bus. Jan
