Re: [Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist code

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.08.2020 11:45, Roger Pau Monné wrote:
> On Thu, Aug 13, 2020 at 09:10:31AM +0100, Paul Durrant wrote:
>>> -----Original Message-----
>>> From: Xen-devel <xen-devel-bounces@xxxxxxxxxxxxxxxxxxxx> On Behalf Of David 
>>> Woodhouse
>>> Sent: 11 August 2020 14:25
>>> To: Paul Durrant <paul.durrant@xxxxxxxxxx>; xen-devel@xxxxxxxxxxxxxxxxxxxx; 
>>> Roger Pau Monne
>>> <roger.pau@xxxxxxxxxx>
>>> Cc: Eslam Elnikety <elnikety@xxxxxxxxx>; Andrew Cooper 
>>> <andrew.cooper3@xxxxxxxxxx>; Shan Haitao
>>> <haitao.shan@xxxxxxxxx>; Jan Beulich <jbeulich@xxxxxxxx>
>>> Subject: Re: [Xen-devel] [PATCH v2] x86/hvm: re-work viridian APIC assist 
>>> code
>>>
>>> Resending this straw man patch at Roger's request, to restart discussion.
>>>
>>> Redux: In order to cope with the relatively rare case of unmaskable
>>> legacy MSIs, each vlapic EOI takes a domain-global spinlock just to
>>> iterate over all IRQs and determine that there's actually nothing to
>>> do.
>>>
>>> In my testing, I observe that this drops Windows performance on passed-
>>> through NVMe from 2.2M IOPS down to about 1.0M IOPS.
>>>
>>> I have a variant of this patch which just has a single per-domain "I
>>> attached legacy unmaskable MSIs" flag, which is never cleared. The
>>> patch below is per-vector (but Roger points out it should be per-vCPU
>>> per-vector). I don't know that we really care enough to do more than
>>> the single per-domain flag, which in real life would never happen
>>> anyway unless you have crappy hardware, at which point you get back to
>>> today's status quo.
>>>
>>> My main concern is that this code is fairly sparsely documented and I'm
>>> only 99% sure that this code path really *is* only for unmaskable MSIs,
>>> and doesn't have some other esoteric use. A second opinion on that
>>> would be particularly welcome.
>>>
>>
>> The loop appears to be there to handle the case where multiple
>> devices assigned to a domain have MSIs programmed with the same
>> dest/vector... which seems like an odd thing for a guest to do but I
>> guess it is at liberty to do it. Does it matter whether they are
>> maskable or not?
> 
> Such configuration would never work properly, as lapic vectors are
> edge triggered and thus can't be safely shared between devices?

Wait - there are two aspects here: Vectors are difficult to be shared
on the same CPU (but it's not impossible if the devices and their
drivers meet certain conditions). But the bitmap gets installed as a
per-domain rather than a per-vcpu one, and using the same vector on
different CPUs is definitely possible, as demonstrated by both Xen
itself as well as Linux.

Jan