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

[Marek Marczykowski-Górecki <marmarek@xxxxxxxxxxxxxxxxxxxxxx>]

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. 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. 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
 - add iommu=intremap=require to refuse boot if intremap is
   missing/broken

[1] 
https://invisiblethingslab.com/resources/2011/Software%20Attacks%20on%20Intel%20VT-d.pdf

-- 
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab

Attachment: signature.asc
Description: PGP signature