On Thu, Mar 01, 2018 at 06:54:01PM +0000, Stephen Bates wrote: > Thanks for the detailed review Bjorn! > > >> + Enabling this option will also disable ACS on all ports behind > >> + any PCIe switch. This effictively puts all devices behind any > >> + switch into the same IOMMU group. > > > Does this really mean "all devices behind the same Root Port"? > > Not necessarily. You might have a cascade of switches (i.e switches > below a switch) to achieve a very large fan-out (in an NVMe SSD > array for example) and we will only disable ACS on the ports below > the relevant switch. The question is what the relevant switch is. We call pci_enable_acs() on every PCI device, including Root Ports. It looks like this relies on get_upstream_bridge_port() to filter out some things. I don't think get_upstream_bridge_port() is doing the right thing, so we need to sort that out first, I guess. > > What does this mean in terms of device security? I assume it means, > > at least, that individual devices can't be assigned to separate VMs. > > This was discussed during v1 [1]. Disabling ACS on all downstream > ports of the switch means that all the EPs below it have to part of > the same IOMMU grouping. However it was also agreed that as long as > the ACS disable occurred at boot time (which is does in v2) then the > virtualization layer will be aware of it and will perform the IOMMU > group formation correctly. > > > I don't mind admitting that this patch makes me pretty nervous, and I > > don't have a clear idea of what the implications of this are, or how > > to communicate those to end users. "The same IOMMU group" is a pretty > > abstract idea. > > Alex gave a good overview of the implications in [1]. > > [1] https://marc.info/?l=linux-pci&m=151512320031739&w=2 This might be a good start, but whatever the implications are, they need to be distilled and made clear directly in the changelog and the Kconfig help text. Bjorn
