Re: [Xen-devel] [PATCH v2] IOMMU: make DMA containment of quarantined devices optional

[Jan Beulich <jbeulich@xxxxxxxx>]

On 13.12.2019 14:11, Jürgen Groß wrote:
> 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.

An admin not wanting to know is, to me, the same as them not wanting
to know about security issues, and hence not subscribing to our
announcements lists. I can accept this being a reasonable thing to
do when it is an _informed_ decision. But with the current
arrangements there's no way whatsoever for an admin to know.

Furthermore, once we ship a release with the defaults as they
currently are, changing the defaults again may be perceived as a
regression, which I think we should (want to) avoid.

> In case we have a successful OSStest run soon I planned to do the
> release without this patch.

I very much disagree - the patch shouldn't have been put in without
having heard back from Kevin. But yes, you're the release manager.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel