> -----Original Message----- > From: Xen-devel <xen-devel-boun...@lists.xenproject.org> On Behalf Of Jan > Beulich > Sent: 13 December 2019 12:53 > To: xen-devel@lists.xenproject.org > Cc: Juergen Gross <jgr...@suse.com>; Kevin Tian <kevin.t...@intel.com>; > Stefano Stabellini <sstabell...@kernel.org>; Julien Grall > <jul...@xen.org>; Wei Liu <w...@xen.org>; Konrad Wilk > <konrad.w...@oracle.com>; George Dunlap <george.dun...@eu.citrix.com>; > Andrew Cooper <andrew.coop...@citrix.com>; Paul Durrant <p...@xen.org>; > Ian Jackson <ian.jack...@citrix.com>; Roger Pau Monné > <roger....@citrix.com> > Subject: [Xen-devel] [PATCH v2] IOMMU: make DMA containment of quarantined > devices optional > > 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 <jbeul...@suse.com> > --- > 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 happy to take better suggestions to replace "full".
How about simply "sink", since that's what it does? [snip] > --- a/xen/drivers/passthrough/iommu.c > +++ b/xen/drivers/passthrough/iommu.c > @@ -30,13 +30,17 @@ bool_t __initdata iommu_enable = 1; > bool_t __read_mostly iommu_enabled; > bool_t __read_mostly force_iommu; > bool_t __read_mostly iommu_verbose; > -bool __read_mostly iommu_quarantine = true; > bool_t __read_mostly iommu_igfx = 1; > bool_t __read_mostly iommu_snoop = 1; > bool_t __read_mostly iommu_qinval = 1; > bool_t __read_mostly iommu_intremap = 1; > bool_t __read_mostly iommu_crash_disable; > > +#define IOMMU_quarantine_none 0 > +#define IOMMU_quarantine_basic 1 > +#define IOMMU_quarantine_full 2 > +uint8_t __read_mostly iommu_quarantine = IOMMU_quarantine_basic; If we have 'IOMMU_quarantine_sink' instead of 'IOMMU_quarantine_full', then how about 'IOMMU_quarantine_write_fault' instead of 'IOMMU_quarantine_basic'? Paul _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel