From: "Marek Marczykowski-Górecki" <marmarek@invisiblethingslab.com>
To: Jan Beulich <jbeulich@suse.com>
Cc: "xen-devel@lists.xenproject.org" <xen-devel@lists.xenproject.org>,
Kevin Tian <kevin.tian@intel.com>,
Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH] VT-d: Tylersburg errata apply to further steppings
Date: Tue, 3 Aug 2021 15:01:23 +0200 [thread overview]
Message-ID: <YQk+I2FglGrnuyPw@mail-itl> (raw)
In-Reply-To: <ec1cc65d-5a10-7ef3-2643-622302cdafb7@suse.com>
[-- Attachment #1: Type: text/plain, Size: 3199 bytes --]
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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
next prev parent reply other threads:[~2021-08-03 13:01 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-08-03 11:13 [PATCH] VT-d: Tylersburg errata apply to further steppings Jan Beulich
2021-08-03 12:21 ` Marek Marczykowski-Górecki
2021-08-03 12:29 ` Jan Beulich
2021-08-03 13:01 ` Marek Marczykowski-Górecki [this message]
2021-08-03 13:06 ` Jan Beulich
2021-08-03 13:12 ` Marek Marczykowski-Górecki
2021-08-03 13:16 ` Jan Beulich
2021-08-03 13:30 ` Marek Marczykowski-Górecki
2021-08-03 13:44 ` Jan Beulich
2021-08-03 13:52 ` Marek Marczykowski-Górecki
2021-08-17 3:02 ` Tian, Kevin
2021-08-18 11:32 ` Andrew Cooper
2021-08-18 12:02 ` Jan Beulich
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YQk+I2FglGrnuyPw@mail-itl \
--to=marmarek@invisiblethingslab.com \
--cc=andrew.cooper3@citrix.com \
--cc=jbeulich@suse.com \
--cc=kevin.tian@intel.com \
--cc=xen-devel@lists.xenproject.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.