Re: [PATCH 3/5] x86/vmsi: use the newly introduced EOI callbacks

[Jan Beulich <jbeulich@xxxxxxxx>]

On 24.08.2020 16:44, Roger Pau Monné wrote:
> On Mon, Aug 24, 2020 at 04:06:31PM +0200, Jan Beulich wrote:
>> On 12.08.2020 14:47, Roger Pau Monne wrote:
>>> Remove the unconditional call to hvm_dpci_msi_eoi in vlapic_handle_EOI
>>> and instead use the newly introduced EOI callback mechanism in order
>>> to register a callback for MSI vectors injected from passed through
>>> devices.
>>
>> In patch 2 you merely invoke the callback when the EOI arrived,
>> but you don't clear the callback (unless I've missed something).
>> Here you register the callback once per triggering of the IRQ,
>> without clearing it from the callback itself either.
> 
> It gets cleared on the next call to vlapic_set_irq_callback, or set to
> a new callback value if the passed callback is not NULL.
> 
>> Why is it
>> correct / safe to keep the callback registered?
> 
> The next vector injected is going to clear it, so should be safe, as
> no vector can be injected without calling vlapic_set_irq_callback.

But what about duplicate EOIs, even if just by bug or erratum?
I notice, for example, that VMX'es VMEXIT handler directly
calls vlapic_handle_EOI(). I'd find it more safe if an
unexpected EOI didn't get any callback invoked.

>> What about
>> conflicting callbacks for shared vectors?
> 
> In this callback model for vlapic only the last injected vector
> callback would get executed. It's my understanding that lapic
> vectors cannot be safely shared unless there's a higher level
> interrupt controller (ie: an IO-APIC) that does the sharing.

As said on a different, but pretty recent thread: I do think
sharing is possible if devices and drivers meet certain criteria
(in particular have a way to determine which of the devices
actually require service). It's just not something one would
normally do. But iirc such a model was used in good old DOS to
overcome the 15 IRQs limit (of which quite a few had fixed
purposes, so for add-in devices there were only very few left).

Jan