Re: [PATCH] VT-d: Tylersburg errata apply to further steppings

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.08.2021 14:21, Marek Marczykowski-Górecki wrote:
> On Tue, Aug 03, 2021 at 01:13:40PM +0200, Jan Beulich wrote:
>> While for 5500 and 5520 chipsets only B3 and C2 are mentioned in the
>> spec update, X58's also mentions B2, and searching the internet suggests
>> systems with this stepping are actually in use. Even worse, for X58
>> erratum #69 is marked applicable even to C2. Split the check to cover
>> all applicable steppings and to also report applicable errata numbers in
>> the log message. The splitting requires using the DMI port instead of
>> the System Management Registers device, but that's then in line (also
>> revision checking wise) with the spec updates.
>>
>> Fixes: 6890cebc6a98 ("VT-d: deal with 5500/5520/X58 errata")
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> ---
>> As to disabling just interrupt remapping (as the initial version of the
>> original patch did) vs disabling the IOMMU as a whole: Using a less
>> heavy workaround would of course be desirable, but then we need to
>> ensure not to misguide the tool stack about the state of the system. It
>> uses the PHYSCAP_directio sysctl output to determine whether PCI pass-
>> through can be made use of, yet that flag is driven by "iommu_enabled"
>> alone, without regard to the setting of "iommu_intremap".
> 
> How does it differ from the situation where interrupt remapping actually
> isn't supported at all? Toolstack will use IOMMU then, in a way that is
> supported on a given platform. Sure, missing interrupt remapping makes
> it less robust[1]. But really, broken and missing interrupt remapping
> should be treated the same way.

I agree; in fact I meant to mention this aspect but then forgot.

> If we would have an option (in
> toolstack, or Xen) to force interrupt remapping, then indeed when it's
> broken, PCI passthrough should be refused (or maybe even system should
> refuse to boot if we'd have something like iommu=intremap=require). But
> none of those actually exists.

"iommu=force" actually does prevent boot from completing when
interrupt remapping is available, but then gets turned off for
some reason. See iommu_setup()'s

    bool_t force_intremap = force_iommu && iommu_intremap;

> And disabling the whole IOMMU in some
> cases of unusable intremap, but not the others, is not exactly useful
> thing to do (it breaks some cases, but still doesn't allow to reason
> about intremap in toolstack).
> 
> So, I propose to disable just iommu_intremap if it's broken as part of
> this bug fix. But, independently (and _not_ as a pre-requisite) do
> either:
>  - let the toolstack know if intremap is used, or

I don't follow why you even emphasize the "not" on this being a prereq.
I consider it a plain bug (with possibly a security angle) that PCI
pass-through may be permitted by the tool stack in the absence of
interrupt remapping, without an explicit admin request to enable this
(even) less secure mode of operation. Not making this a prereq would
mean to widen the scope of the bug.

Jan