[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v2] IOMMU: make DMA containment of quarantined devices optional
- To: Jan Beulich <jbeulich@xxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- From: Jürgen Groß <jgross@xxxxxxxx>
- Date: Fri, 13 Dec 2019 14:11:25 +0100
- Cc: Kevin Tian <kevin.tian@xxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Wei Liu <wl@xxxxxxx>, Konrad Wilk <konrad.wilk@xxxxxxxxxx>, George Dunlap <George.Dunlap@xxxxxxxxxxxxx>, Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Paul Durrant <paul@xxxxxxx>, Ian Jackson <ian.jackson@xxxxxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
- Delivery-date: Fri, 13 Dec 2019 13:11:33 +0000
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 13.12.19 13:53, Jan Beulich wrote:
Containing still in flight DMA was introduced to work around certain
devices / systems hanging hard upon hitting an IOMMU fault. Passing
through (such) devices (on such systems) is inherently insecure (as
guests could easily arrange for IOMMU faults to occur). Defaulting to
a mode where admins may not even become aware of issues with devices can
be considered undesirable. Therefore convert this mode of operation to
an optional one, not one enabled by default.
This involves resurrecting code commit ea38867831da ("x86 / iommu: set
up a scratch page in the quarantine domain") did remove, in a slightly
extended and abstracted fashion. Here, instead of reintroducing a pretty
pointless use of "goto" in domain_context_unmap(), and instead of making
the function (at least temporarily) inconsistent, take the opportunity
and replace the other similarly pointless "goto" as well.
In order to key the re-instated bypasses off of there (not) being a root
page table this further requires moving the allocate_domain_resources()
invocation from reassign_device() to amd_iommu_setup_domain_device() (or
else reassign_device() would allocate a root page table anyway); this is
benign to the second caller of the latter function.
Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
---
As far as 4.13 is concerned, I guess if we can't come to an agreement
here, the only other option is to revert ea38867831da from the branch,
for having been committed prematurely (I'm not so much worried about the
master branch, where we have ample time until 4.14). What I surely want
to see us avoid is a back and forth in behavior of released versions.
(Note that 4.12.2 is similarly blocked on a decision either way here.)
I'm not really sure we really need to revert ea38867831da before the
4.13 release. It might not be optimal, but I'm quite sure the number of
cases where this could be an issue is rather small already, and I tend
to agree with Paul that admins who really care will more likely want to
select the option where the system will "just work". IMO the "noticeable
failure" is something which will be selected mostly by developers. But
I'm not an expert in that area, so I don't want to influence the
decision regarding the to be selected default too much.
In case we have a successful OSStest run soon I planned to do the
release without this patch.
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
