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

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

On Tue, Aug 03, 2021 at 02:29:01PM +0200, Jan Beulich wrote:
> On 03.08.2021 14:21, Marek Marczykowski-Górecki wrote:
> > 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;

Ok, then, just setting iommu_intremap=false should do the right thing,
if platform_quirks_init() is called somewhere between the above line,
and actual enforcement of iommu=force few lines later. I couldn't
quickly find if that is the case - is it?

Anyway, this still doesn't give the toolstack, or the admin sufficient
control, because there is no way to express "use PCI passthrough only if
IOMMU _and_ interrupt remapping is in use". Even with iommu=force,
because intremap could simply be missing on the platform. So, to be
sure, the admin still need to inspect the boot log to fish that
information out - could do that in the "intremap broken" case as well.

So, iommu=force should either always require intremap too (IMO less
preferable), or there should be separate intremap=force, that prevents
the boot if intremap cannot be used for any reason. Even better, if the
toolstack could figure it out, and apply the admin policy on per-domain
basis, but that's a broader change (that IMO should not be a part of a
bugfix).

> > 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.

As explained above - the scope here doesn't really matter. Admin
currently (with or without this commit) cannot rely on intremap being
used, even with iommu=force. For that, admin needs to inspect the boot
log. And when done, inspecting the boot log will catch both cases -
intremap missing and intremap broken. But, disabling the whole IOMMU if
intremap is broken, doesn't even allow to make a conscious choice to
choose to use it. This breaks the (very much valid) configuration of
running a _trusted_ HVM guest with PCI passthorugh, on some platforms.

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

Attachment: signature.asc
Description: PGP signature