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

[Jan Beulich <jbeulich@xxxxxxxx>]

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