Re: [PATCH v2] VTD/Intremap: Disable Intremap on Chipset 5500/5520/X58 due to errata

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

On 05/03/13 07:45, Jan Beulich wrote:
>>>> On 01.03.13 at 14:46, Malcolm Crossley <malcolm.crossley@citrix.com> wrote:
>> http://www.intel.com/content/www/us/en/chipsets/5520-and-5500-chipset-ioh-specific 
>> ation-update.html
>>
>> Stepping B-3 has two errata (#47 and #53) related to Interrupt
>> remapping, to which the workaround is for the BIOS to completely disable
>> interrupt remapping.  These errata are fixed in stepping C-2.
>>
>> Unfortunately this chipset stepping is very common and many BIOSes are
>> not disabling interrupt remapping on this stepping .  We can detect this in
>> Xen and prevent Xen from using the problematic interrupt remapping feature.
>>
>> The Intel 5500/5520/X58 chipset does not support VT-d
>> Extended Interrupt Mode(EIM). This means the iommu_supports_eim() check
>> always fails and so x2apic mode cannot be enabled in Xen before this quirk
>> disables the interrupt remapping feature.
> Just like for the first version of this patch (where you didn't really
> follow up on all the comment made), and in line with the XSA-36
> follow up that you also asked upon some time last week, I don't
> think we can take this as is. First and foremost we need to settle
> on a policy:
>
> - fully disable IOMMU
>   - stay with allowing PV passthrough in this mode
>   - also suppress PV passthrough in this mode
> - partially disable IOMMU in presence of platform errata
>   (yielding an unexpectedly - to the user - insecure system)
> - other options?

(Replying on behalf of Malcolm who is out of the office this week)

There is only one satisfactory option, and this is to provide a report
to the toolstack of what hardware is available/missing/buggy.  The
toolstack can then make an informed decision as to whether to use
passthrough or not.

The specific usecase we have in mind going forward is with driver
domains.  Even without interrupt remapping, it is safe to pass devices
through to trusted code base driver domains.  A binary switch on the Xen
command line is not acceptable nor appropriate in this situation.

>
> All that including ways to override the behavior towards lower
> security (but perhaps not towards erroneous behavior, i.e. in the
> case here we probably shouldn't permit interrupt remapping to be
> forcibly enabled).
>
> This specifically needs to be distinguished from firmware
> disabling/hiding some functionality - we ought to be permitted
> to expect the operator to know the security level that the
> platform provides.

Why? This patch has the *exactly the same effect* as a BIOS upgrade
which follows the erratum recommendations.  The difference being that it
is at least tweakable on the Xen command line.  The real problem with
this patch is because of how common these chipsets are, and the fact
that no BIOSes we are aware of have implemented the erratum workaround.

This bug is the root cause of at least 7 customer escalations of weird
crashes, and suspected cause of 5 more.  Customers do not need to be
actively using PCI passthrough to have this issue screw with dom0.

>
> My personal opinion is that in the event where we need to disable
> _any_ IOMMU functionality, we ought to _fully_ suppress
> passthrough (and hence we can as well disable the IOMMU
> altogether, as we did for XSA-36). We can _then_ allow the user
> to re-enable the IOMMU (in the case here as much as for XSA-36
> e.g. via "iommu=no-intremap", i.e. the operator explicitly
> declaring to be willing to take the risk). That would require parts
> of the XSA-36 follow up patch that I had posted (other parts of it
> would need adjustment).

See the previously device driver usercase.

Also, this is unacceptable for any business usecase of Xen.  I realize
that this is our problem rather than upstreams, but Citrix is certainly
not alone in this regard.  We cannot push a bugfix or security which
regresses functionality, which is why XSA-36 is not currently in an
acceptable state.  It is the same reasoning as your commit for c/s
25765:e6ca45ca03c2

>
> Only then can we, consistently for AMD Vi and VT-d, implement
> workarounds like the one here.
>
> Besides that, please don't use dprintk() except for truly
> debugging messages - the file/line prefixing is only producing
> extra noise when the message itself is unique.
>
> And - do you really need to iterate over all buses on segment 0?
> The X58 data sheet says at the top of section 17.1: "All devices
> on the IOH reside on bus 0". I wonder whether you wouldn't
> instead need to do this over all segments, on each bus 0.
>
> Jan
>

The chipsets do not support multi-segment systems, and we have a
multi-socket affected systems with multiple of these chipsets, with none
of the IOH's on bus 0.

~Andrew