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
> at least, that individual devices can't be assigned to separate VMs.
This was discussed during v1 . 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 .
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.