Re: [PATCH 0/4] x86/MSI-X: XSA-120 follow-up

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 10/03/15 16:16, Jan Beulich wrote:
> The problem requiring the first patch here is actually what lead to
> XSA-120.
>
> 1: be more careful during teardown
> 2: access MSI-X table only after having enabled MSI-X
> 3: reduce fiddling with control register during restore
> 4: cleanup
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Having taken another look through I am still hesitant, but don't have a
concrete issue to object to.

This code is used extensively in the idt vector migration codepath, and
in irq->ack() functions.  I still fear that the additional config space
accesses will come with a non-neglegable overhead.

One issue, orthogonal to these changes, which XenServer has hit before
(a fairly long time ago) is having a device failure and having it fall
completely off the PCI bus (the firmware helpfully ate the error).  At
that point, config cycles started returning ~0's, and I seem to recall
that Xen suffered a cascade failure as it started walking backwards
along the msi entry array.  The underlying problem here is Xen's
reliance to read everything from config space, given the lack of
exclusion against dom0 having access.

Given these extra config accesses, I am seriously wondering whether it
would be more efficient overall to trap&emulate dom0 completely and
allow Xen to cache things like the current control register state,
locations of capability structures, etc.  I genuinely don't know the
answer, but I suspect the comparative expense of config accesses means
that we don't need to replace many of the lookups for a net benefit. 
(Of course, it doesn't protect against backdoor access, but all bets are
already off at that point.)

~Andrew