RFC on No ACS Support and SMMUv3 Support

RFC on No ACS Support and SMMUv3 Support

[Sinan Kaya]

Hi,
I am getting ready to submit a quirk patch. Before I go ahead and submit
it for review, I wanted to get ARM IOMMU developers and PCI/VFIO maintainers
together to figure out what the best approach would be.

The Qualcomm QDF2400 server does not support the ACS functionality. 
The server chip implements the ARM SMMUv3 specification with Stream Id = BDF. 

SMMUv3 allows each PCIe function to have unique SMMU mappings and provides
some level of isolation. 

With the current upstream SMMUv3 driver, we are unable to do a passthrough
for a virtual function. This is caused by the pci_device_group() function's
failure to find the smallest isolation group in pci_acs_path_enabled()
function. 

Since no group is found, all the PCI functions are placed into the same
group at the end of the function. 

There are numerous quirk patches when it comes to ACS.

pci_quirk_amd_sb_acs()
pci_quirk_cavium_acs()
pci_quirk_intel_pch_acs_match()

These quirk patches allow a group to be generated per PCI function. Everything
works fine.

I can go ahead and add 

pci_quirk_qualcomm_acs() with the same contents as pci_quirk_cavium_acs()

Tomorrow, some other ARM64 vendors like up and start adding pci_quirk_vendor_acs()
functions. IMO, it leads to unnecessary code duplication. 

I can go ahead and try to make the patches similar with generic names like pci_quirk_no_acs()
so that every vendor can use to it. Still, it would require some quirk patch from a vendor.

Nate has been changing the arm_smmu_device_group() function internally to always
use generic_device_group(). This provides support for the Virtual Function passthrough
but it is not future proof. Tomorrow, some HW with ACS capability can show up and
the ARM SMMUv3 driver wouldn't bother to see if there is actual HW isolation path
available.

What I really would like to do is to find a balance between what Nate has been doing
internally and what pci_device_group() does by changing the SMMUv3 driver to query if
ACS path is supported or not similar to the loop in pci_device_group. If not supported,
fallback to generic_device_group() function in arm_smmu_device_group().

I'm just looking for a more permanent solution rather than relying on quirks for chips
A, B, C and D.

Please let me know the preferred path.

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: RFC on No ACS Support and SMMUv3 Support

[Alex Williamson]

On Mon, 13 Feb 2017 17:24:40 -0500
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi,
> I am getting ready to submit a quirk patch. Before I go ahead and submit
> it for review, I wanted to get ARM IOMMU developers and PCI/VFIO maintainers
> together to figure out what the best approach would be.
> 
> The Qualcomm QDF2400 server does not support the ACS functionality. 
> The server chip implements the ARM SMMUv3 specification with Stream Id = BDF. 
> 
> SMMUv3 allows each PCIe function to have unique SMMU mappings and provides
> some level of isolation. 

"some level"?  Regardless, this is irrelevant.  ACS is how PCIe
describes the specific forwarding behavior of transactions within the
PCIe hierarchy.  The granularity of the SMMU translation is different
from ACS.

> With the current upstream SMMUv3 driver, we are unable to do a passthrough
> for a virtual function. This is caused by the pci_device_group() function's
> failure to find the smallest isolation group in pci_acs_path_enabled()
> function. 
> 
> Since no group is found, all the PCI functions are placed into the same
> group at the end of the function. 
> 
> There are numerous quirk patches when it comes to ACS.
> 
> pci_quirk_amd_sb_acs()
> pci_quirk_cavium_acs()
> pci_quirk_intel_pch_acs_match()
> 
> These quirk patches allow a group to be generated per PCI function. Everything
> works fine.
> 
> I can go ahead and add 
> 
> pci_quirk_qualcomm_acs() with the same contents as pci_quirk_cavium_acs()

Creating a quirk is not simply a matter of making the code do what you
want, an ACS quirk is you, on behalf of Qualcomm, vouching for the
hardware behaving in a certain way.  Specifically you're saying that
the device does Source Validation (ie. a downstream device cannot spoof
translations for devices on a different bus), the device does not do
Request Redirection (ie. peer-to-peer shortcuts that may bypass the
IOMMU), the device does not do Completion Redirection (same, but for
completions), and the device does Upstream Forwarding (ie. no
shortcuts).  If the hardware does not honor these behaviors then adding
a quirk is only putting customers at risk.

> Tomorrow, some other ARM64 vendors like up and start adding pci_quirk_vendor_acs()
> functions. IMO, it leads to unnecessary code duplication. 

Which is exactly why they should be implementing ACS in their hardware!

> I can go ahead and try to make the patches similar with generic names like pci_quirk_no_acs()
> so that every vendor can use to it. Still, it would require some quirk patch from a vendor.
> 
> Nate has been changing the arm_smmu_device_group() function internally to always
> use generic_device_group(). This provides support for the Virtual Function passthrough
> but it is not future proof. Tomorrow, some HW with ACS capability can show up and
> the ARM SMMUv3 driver wouldn't bother to see if there is actual HW isolation path
> available.
> 
> What I really would like to do is to find a balance between what Nate has been doing
> internally and what pci_device_group() does by changing the SMMUv3 driver to query if
> ACS path is supported or not similar to the loop in pci_device_group. If not supported,
> fallback to generic_device_group() function in arm_smmu_device_group().

Intel VT-d and AMD-Vi already manage to do this, what is it about an
SMMU topology that makes it significantly different.  Note that like
SMMU, both of these have BDF level granularity for identifying devices,
but defer to the ACS based isolation for grouping based on downstream
components.  The best IOMMU available cannot overcome transactions that
it never sees because they're rerouted before reaching the IOMMU.

> I'm just looking for a more permanent solution rather than relying on quirks for chips
> A, B, C and D.

Implement ACS in all downstream ports and multifunction endpoints!
Thanks,

Alex

Re: RFC on No ACS Support and SMMUv3 Support

[Sinan Kaya]

Hi Alex,

Thanks for your response. Other comments inline.

On 2/13/2017 6:06 PM, Alex Williamson wrote:
> On Mon, 13 Feb 2017 17:24:40 -0500
> Sinan Kaya <okaya@codeaurora.org> wrote:
> 
>> Hi,
>> I am getting ready to submit a quirk patch. Before I go ahead and submit
>> it for review, I wanted to get ARM IOMMU developers and PCI/VFIO maintainers
>> together to figure out what the best approach would be.
>>
>> The Qualcomm QDF2400 server does not support the ACS functionality. 
>> The server chip implements the ARM SMMUv3 specification with Stream Id = BDF. 
>>
>> SMMUv3 allows each PCIe function to have unique SMMU mappings and provides
>> some level of isolation. 
> 
> "some level"?  Regardless, this is irrelevant.  ACS is how PCIe
> describes the specific forwarding behavior of transactions within the
> PCIe hierarchy.  The granularity of the SMMU translation is different
> from ACS.

Fair enough. SMMU just protects addresses that are not mapped. SMMU won't protect
any transactions happening inside the PCIe hierarchy. The likelihood of common
IO addresses varies from application to application.

> 
>> With the current upstream SMMUv3 driver, we are unable to do a passthrough
>> for a virtual function. This is caused by the pci_device_group() function's
>> failure to find the smallest isolation group in pci_acs_path_enabled()
>> function. 
>>
>> Since no group is found, all the PCI functions are placed into the same
>> group at the end of the function. 
>>
>> There are numerous quirk patches when it comes to ACS.
>>
>> pci_quirk_amd_sb_acs()
>> pci_quirk_cavium_acs()
>> pci_quirk_intel_pch_acs_match()
>>
>> These quirk patches allow a group to be generated per PCI function. Everything
>> works fine.
>>
>> I can go ahead and add 
>>
>> pci_quirk_qualcomm_acs() with the same contents as pci_quirk_cavium_acs()
> 

> Creating a quirk is not simply a matter of making the code do what you
> want, an ACS quirk is you, on behalf of Qualcomm, vouching for the
> hardware behaving in a certain way.  

Sure, let me clarify what the hardware does below.

> Specifically you're saying that
> the device does Source Validation (ie. a downstream device cannot spoof
> translations for devices on a different bus), 

Correct. Hardware supports source validation but it will report the issue
as Completer Abort instead of ACS Violation. Also, each root port is a different
segment on this particular design. 

> the device does not do
> Request Redirection (ie. peer-to-peer shortcuts that may bypass the
> IOMMU), the device does not do Completion Redirection (same, but for
> completions), 

Hardware doesn't support peer-to-peer and no segment sharing across root ports
unlike Intel architecture where most of the devices are sitting on segment 0
with multiple bridges/switch.

> and the device does Upstream Forwarding (ie. no
> shortcuts).  If the hardware does not honor these behaviors then adding
> a quirk is only putting customers at risk.

No p2p again. The assumption was that p2p would be possible with a PCIe switch
connected to a root port but we missed the fact that there are security implications
required by the ACS implementation. Note that this is ACS requirement is a Linux
statement not PCIe spec.

> 
>> Tomorrow, some other ARM64 vendors like up and start adding pci_quirk_vendor_acs()
>> functions. IMO, it leads to unnecessary code duplication. 
> 
> Which is exactly why they should be implementing ACS in their hardware!
> 

Note that I'm not disagreeing with you or telling that ACS is unnecessary and SMMU
has a bigger hammer etc. I now understand the value of ACS and what it takes to build
a secure peer-to-peer system. I also want to put a big fat warning about the fact that
we are sharing the group to the next SOC designer's attention.

My only concern is the fact that the QDF2400 and QDF2432 missed this requirement and we have
SoC out in the field. I wish this requirement was explicit somewhere like PCI/ACPI
documentation.

I'll make sure that the next chip gets this feature. This is an iterative process with
lessons learnt on the way.

>> I can go ahead and try to make the patches similar with generic names like pci_quirk_no_acs()
>> so that every vendor can use to it. Still, it would require some quirk patch from a vendor.
>>
>> Nate has been changing the arm_smmu_device_group() function internally to always
>> use generic_device_group(). This provides support for the Virtual Function passthrough
>> but it is not future proof. Tomorrow, some HW with ACS capability can show up and
>> the ARM SMMUv3 driver wouldn't bother to see if there is actual HW isolation path
>> available.
>>
>> What I really would like to do is to find a balance between what Nate has been doing
>> internally and what pci_device_group() does by changing the SMMUv3 driver to query if
>> ACS path is supported or not similar to the loop in pci_device_group. If not supported,
>> fallback to generic_device_group() function in arm_smmu_device_group().
> 

My first goal is to support virtual function passthrough for device's that are directly
connected. This will be possible with the quirk I proposed and it will be the most
secure solution. It can certainly be generalized for other systems.

My second goal is extend the code such that ACS validation is up to the customer via 
pci=noacs kernel command line for instance. This will let the customer choose what he
really wants rather than kernel trying to be smart and protective. By passing pci=noacs
parameter, customer acknowledges the risks this command line carries.

> Intel VT-d and AMD-Vi already manage to do this, what is it about an
> SMMU topology that makes it significantly different.  Note that like
> SMMU, both of these have BDF level granularity for identifying devices,
> but defer to the ACS based isolation for grouping based on downstream
> components.  The best IOMMU available cannot overcome transactions that
> it never sees because they're rerouted before reaching the IOMMU.

Sorry for the wrong pointers. I was not debating what ARM SMMUv3 does vs. what other
architectures does in HW. 

> 
>> I'm just looking for a more permanent solution rather than relying on quirks for chips
>> A, B, C and D.
> 
> Implement ACS in all downstream ports and multifunction endpoints!
> Thanks,

Understood. This should have been explicit somewhere like PCIE Appendix of the SBSA
or some Documentation in Linux.

I'll make sure that I put that big fat warning there.

> 
> Alex
> 

Sinan

-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: RFC on No ACS Support and SMMUv3 Support

[Alex Williamson]

On Mon, 13 Feb 2017 19:14:31 -0500
Sinan Kaya <okaya@codeaurora.org> wrote:

> Hi Alex,
> 
> Thanks for your response. Other comments inline.
> 
> On 2/13/2017 6:06 PM, Alex Williamson wrote:
> > On Mon, 13 Feb 2017 17:24:40 -0500
> > Sinan Kaya <okaya@codeaurora.org> wrote:
> >   
> >> Hi,
> >> I am getting ready to submit a quirk patch. Before I go ahead and submit
> >> it for review, I wanted to get ARM IOMMU developers and PCI/VFIO maintainers
> >> together to figure out what the best approach would be.
> >>
> >> The Qualcomm QDF2400 server does not support the ACS functionality. 
> >> The server chip implements the ARM SMMUv3 specification with Stream Id = BDF. 
> >>
> >> SMMUv3 allows each PCIe function to have unique SMMU mappings and provides
> >> some level of isolation.   
> > 
> > "some level"?  Regardless, this is irrelevant.  ACS is how PCIe
> > describes the specific forwarding behavior of transactions within the
> > PCIe hierarchy.  The granularity of the SMMU translation is different
> > from ACS.  
> 
> Fair enough. SMMU just protects addresses that are not mapped. SMMU won't protect
> any transactions happening inside the PCIe hierarchy. The likelihood of common
> IO addresses varies from application to application.
> 
> >   
> >> With the current upstream SMMUv3 driver, we are unable to do a passthrough
> >> for a virtual function. This is caused by the pci_device_group() function's
> >> failure to find the smallest isolation group in pci_acs_path_enabled()
> >> function. 
> >>
> >> Since no group is found, all the PCI functions are placed into the same
> >> group at the end of the function. 
> >>
> >> There are numerous quirk patches when it comes to ACS.
> >>
> >> pci_quirk_amd_sb_acs()
> >> pci_quirk_cavium_acs()
> >> pci_quirk_intel_pch_acs_match()
> >>
> >> These quirk patches allow a group to be generated per PCI function. Everything
> >> works fine.
> >>
> >> I can go ahead and add 
> >>
> >> pci_quirk_qualcomm_acs() with the same contents as pci_quirk_cavium_acs()  
> >   
> 
> > Creating a quirk is not simply a matter of making the code do what you
> > want, an ACS quirk is you, on behalf of Qualcomm, vouching for the
> > hardware behaving in a certain way.    
> 
> Sure, let me clarify what the hardware does below.
> 
> > Specifically you're saying that
> > the device does Source Validation (ie. a downstream device cannot spoof
> > translations for devices on a different bus),   
> 
> Correct. Hardware supports source validation but it will report the issue
> as Completer Abort instead of ACS Violation. Also, each root port is a different
> segment on this particular design. 
> 
> > the device does not do
> > Request Redirection (ie. peer-to-peer shortcuts that may bypass the
> > IOMMU), the device does not do Completion Redirection (same, but for
> > completions),   
> 
> Hardware doesn't support peer-to-peer and no segment sharing across root ports
> unlike Intel architecture where most of the devices are sitting on segment 0
> with multiple bridges/switch.
> 
> > and the device does Upstream Forwarding (ie. no
> > shortcuts).  If the hardware does not honor these behaviors then adding
> > a quirk is only putting customers at risk.  
> 
> No p2p again. The assumption was that p2p would be possible with a PCIe switch
> connected to a root port but we missed the fact that there are security implications
> required by the ACS implementation. Note that this is ACS requirement is a Linux
> statement not PCIe spec.
> 
> >   
> >> Tomorrow, some other ARM64 vendors like up and start adding pci_quirk_vendor_acs()
> >> functions. IMO, it leads to unnecessary code duplication.   
> > 
> > Which is exactly why they should be implementing ACS in their hardware!
> >   
> 
> Note that I'm not disagreeing with you or telling that ACS is unnecessary and SMMU
> has a bigger hammer etc. I now understand the value of ACS and what it takes to build
> a secure peer-to-peer system. I also want to put a big fat warning about the fact that
> we are sharing the group to the next SOC designer's attention.
> 
> My only concern is the fact that the QDF2400 and QDF2432 missed this requirement and we have
> SoC out in the field. I wish this requirement was explicit somewhere like PCI/ACPI
> documentation.
> 
> I'll make sure that the next chip gets this feature. This is an iterative process with
> lessons learnt on the way.
> 
> >> I can go ahead and try to make the patches similar with generic names like pci_quirk_no_acs()
> >> so that every vendor can use to it. Still, it would require some quirk patch from a vendor.
> >>
> >> Nate has been changing the arm_smmu_device_group() function internally to always
> >> use generic_device_group(). This provides support for the Virtual Function passthrough
> >> but it is not future proof. Tomorrow, some HW with ACS capability can show up and
> >> the ARM SMMUv3 driver wouldn't bother to see if there is actual HW isolation path
> >> available.
> >>
> >> What I really would like to do is to find a balance between what Nate has been doing
> >> internally and what pci_device_group() does by changing the SMMUv3 driver to query if
> >> ACS path is supported or not similar to the loop in pci_device_group. If not supported,
> >> fallback to generic_device_group() function in arm_smmu_device_group().  
> >   
> 
> My first goal is to support virtual function passthrough for device's that are directly
> connected. This will be possible with the quirk I proposed and it will be the most
> secure solution. It can certainly be generalized for other systems.

Why is this anything more than a quirk for the affected PCIe root port
vendor:device IDs and use of pci_device_group() to evaluate the rest of
the topology, as appears is already done?  Clearly a blanket exception
for the platform wouldn't necessarily be correct if a user could plugin
a device that adds a PCIe switch.
 
> My second goal is extend the code such that ACS validation is up to the customer via 
> pci=noacs kernel command line for instance. This will let the customer choose what he
> really wants rather than kernel trying to be smart and protective. By passing pci=noacs
> parameter, customer acknowledges the risks this command line carries.

Be prepared that this will need to taint the kernel since we make
assertions with drivers like vfio to provide secure, isolated user
access to devices and we can't make that statement if the user has
bypassed ACS enforcement.  There is absolutely no way that such an
option would not be severely abused.  In fact, I tried adding such an
option with the pcie_acs_override= patch[1], Bjorn rejected it and I'm
thankful that he did.  I don't think this is a good idea, sometimes the
kernel does need to be smarter than the user to protect them from
themselves.  Any easy bypass also lets hardware vendors ignore the
issue longer.  Thanks,

Alex

[1] https://lkml.org/lkml/2013/5/30/513

Re: RFC on No ACS Support and SMMUv3 Support

[Sinan Kaya]

On 2/13/2017 8:46 PM, Alex Williamson wrote:
>> My first goal is to support virtual function passthrough for device's that are directly
>> connected. This will be possible with the quirk I proposed and it will be the most
>> secure solution. It can certainly be generalized for other systems.
> Why is this anything more than a quirk for the affected PCIe root port
> vendor:device IDs and use of pci_device_group() to evaluate the rest of
> the topology, as appears is already done?  Clearly a blanket exception
> for the platform wouldn't necessarily be correct if a user could plugin
> a device that adds a PCIe switch.

I was going to go this direction first. I wanted to check with everybody to see
if there are other/better alternatives possible via either changing 
pci_device_group or changing the smmuv3 driver.

>  
>> My second goal is extend the code such that ACS validation is up to the customer via 
>> pci=noacs kernel command line for instance. This will let the customer choose what he
>> really wants rather than kernel trying to be smart and protective. By passing pci=noacs
>> parameter, customer acknowledges the risks this command line carries.
> Be prepared that this will need to taint the kernel since we make
> assertions with drivers like vfio to provide secure, isolated user
> access to devices and we can't make that statement if the user has
> bypassed ACS enforcement.  There is absolutely no way that such an
> option would not be severely abused.  In fact, I tried adding such an
> option with the pcie_acs_override= patch[1], Bjorn rejected it and I'm
> thankful that he did.  I don't think this is a good idea, sometimes the
> kernel does need to be smarter than the user to protect them from
> themselves.  Any easy bypass also lets hardware vendors ignore the
> issue longer.  Thanks,

Bjorn, any inputs?

> 
> Alex
> 
> [1] https://lkml.org/lkml/2013/5/30/513
> 


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.

Re: RFC on No ACS Support and SMMUv3 Support

[Will Deacon]

On Mon, Feb 13, 2017 at 08:54:04PM -0500, Sinan Kaya wrote:
> On 2/13/2017 8:46 PM, Alex Williamson wrote:
> >> My first goal is to support virtual function passthrough for device's that are directly
> >> connected. This will be possible with the quirk I proposed and it will be the most
> >> secure solution. It can certainly be generalized for other systems.
> > Why is this anything more than a quirk for the affected PCIe root port
> > vendor:device IDs and use of pci_device_group() to evaluate the rest of
> > the topology, as appears is already done?  Clearly a blanket exception
> > for the platform wouldn't necessarily be correct if a user could plugin
> > a device that adds a PCIe switch.
> 
> I was going to go this direction first. I wanted to check with everybody to see
> if there are other/better alternatives possible via either changing 
> pci_device_group or changing the smmuv3 driver.

Just to echo what Alex has been saying, I really don't think we should
support this type of system by quirking the topology code in the SMMU
driver. The SMMU isn't at fault here; the problems are all upstream of that.
Legitimising non-ACS machines in the SMMU driver gives little incentive for
people to build systems correctly and undermines the security guarantees
that the SMMU (and VFIO) are trying to provide.

I appreciate that I/O virtualisation on arm64 has been a learning curve for
everybody involved, but that's not an excuse for moving the goalposts when
it comes to device isolation.

Will

Re: RFC on No ACS Support and SMMUv3 Support

[Robin Murphy]

On 14/02/17 01:54, Sinan Kaya wrote:
> On 2/13/2017 8:46 PM, Alex Williamson wrote:
>>> My first goal is to support virtual function passthrough for device's that are directly
>>> connected. This will be possible with the quirk I proposed and it will be the most
>>> secure solution. It can certainly be generalized for other systems.
>> Why is this anything more than a quirk for the affected PCIe root port
>> vendor:device IDs and use of pci_device_group() to evaluate the rest of
>> the topology, as appears is already done?  Clearly a blanket exception
>> for the platform wouldn't necessarily be correct if a user could plugin
>> a device that adds a PCIe switch.
> 
> I was going to go this direction first. I wanted to check with everybody to see
> if there are other/better alternatives possible via either changing 
> pci_device_group or changing the smmuv3 driver.

I second Alex's opinion here. The SMMU driver is absolutely not the
appropriate place to address this - it's a PCI issue that needs to be
solved in the PCI domain. To flip things around, regardless of VFIO,
overriding group allocation may just plain break some devices - if you
plug in, say, some USB 2.0 card with an OHCI/EHCI pair on two different
functions, assigning them to different groups such that they get
different domains and can't hand off valid DMA addresses to each other
is liable to go downhill very quickly.

Robin.

>>> My second goal is extend the code such that ACS validation is up to the customer via 
>>> pci=noacs kernel command line for instance. This will let the customer choose what he
>>> really wants rather than kernel trying to be smart and protective. By passing pci=noacs
>>> parameter, customer acknowledges the risks this command line carries.
>> Be prepared that this will need to taint the kernel since we make
>> assertions with drivers like vfio to provide secure, isolated user
>> access to devices and we can't make that statement if the user has
>> bypassed ACS enforcement.  There is absolutely no way that such an
>> option would not be severely abused.  In fact, I tried adding such an
>> option with the pcie_acs_override= patch[1], Bjorn rejected it and I'm
>> thankful that he did.  I don't think this is a good idea, sometimes the
>> kernel does need to be smarter than the user to protect them from
>> themselves.  Any easy bypass also lets hardware vendors ignore the
>> issue longer.  Thanks,
> 
> Bjorn, any inputs?
> 
>>
>> Alex
>>
>> [1] https://lkml.org/lkml/2013/5/30/513
>>
> 
> 

(apologies if a disclaimer appears below; SMTP problems...)

Re: RFC on No ACS Support and SMMUv3 Support

[Will Deacon]

On Mon, Feb 13, 2017 at 08:54:04PM -0500, Sinan Kaya wrote:
> On 2/13/2017 8:46 PM, Alex Williamson wrote:
> >> My first goal is to support virtual function passthrough for device's that are directly
> >> connected. This will be possible with the quirk I proposed and it will be the most
> >> secure solution. It can certainly be generalized for other systems.
> > Why is this anything more than a quirk for the affected PCIe root port
> > vendor:device IDs and use of pci_device_group() to evaluate the rest of
> > the topology, as appears is already done?  Clearly a blanket exception
> > for the platform wouldn't necessarily be correct if a user could plugin
> > a device that adds a PCIe switch.
> 
> I was going to go this direction first. I wanted to check with everybody to see
> if there are other/better alternatives possible via either changing 
> pci_device_group or changing the smmuv3 driver.

Just to echo what Alex has been saying, I really don't think we should
support this type of system by quirking the topology code in the SMMU
driver. The SMMU isn't at fault here; the problems are all upstream of that.
Legitimising non-ACS machines in the SMMU driver gives little incentive for
people to build systems correctly and undermines the security guarantees
that the SMMU (and VFIO) are trying to provide.

I appreciate that I/O virtualisation on arm64 has been a learning curve for
everybody involved, but that's not an excuse for moving the goalposts when
it comes to device isolation.

Will

Re: RFC on No ACS Support and SMMUv3 Support

[Sinan Kaya]

On 2/14/2017 7:36 AM, Will Deacon wrote:
> On Mon, Feb 13, 2017 at 08:54:04PM -0500, Sinan Kaya wrote:
>> On 2/13/2017 8:46 PM, Alex Williamson wrote:
>>>> My first goal is to support virtual function passthrough for device's that are directly
>>>> connected. This will be possible with the quirk I proposed and it will be the most
>>>> secure solution. It can certainly be generalized for other systems.
>>> Why is this anything more than a quirk for the affected PCIe root port
>>> vendor:device IDs and use of pci_device_group() to evaluate the rest of
>>> the topology, as appears is already done?  Clearly a blanket exception
>>> for the platform wouldn't necessarily be correct if a user could plugin
>>> a device that adds a PCIe switch.
>>
>> I was going to go this direction first. I wanted to check with everybody to see
>> if there are other/better alternatives possible via either changing 
>> pci_device_group or changing the smmuv3 driver.
> 
> Just to echo what Alex has been saying, I really don't think we should
> support this type of system by quirking the topology code in the SMMU
> driver. The SMMU isn't at fault here; the problems are all upstream of that.
> Legitimising non-ACS machines in the SMMU driver gives little incentive for
> people to build systems correctly and undermines the security guarantees
> that the SMMU (and VFIO) are trying to provide.
> 
> I appreciate that I/O virtualisation on arm64 has been a learning curve for
> everybody involved, but that's not an excuse for moving the goalposts when
> it comes to device isolation.
> 
> Will
> 

Thanks to everyone for feedback. I'll follow the quirk path as requested. 
I'll be posting the patch soon.


-- 
Sinan Kaya
Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc.
Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.