Re: [RFC] PCI ACS Flags Clarification

[Alex Williamson]

On Wed, 5 Apr 2017 09:51:06 -0400
Sinan Kaya  wrote:

> Hi Alex,
> 
> Thank you very much for the detailed explanation.
> 
> On 4/4/2017 3:39 PM, Alex Williamson wrote:
> > On Tue, 4 Apr 2017 14:47:58 -0400
> > Sinan Kaya  wrote:
> >   
> [cut]
> 
> >> The requirement is to have an 
> >> 1. ACS capability with PCI_ACS_SV if p2p is not supported
> >> 2. ACS capability with PCI_ACS_SV|PCI_ACS_RR | PCI_ACS_CR |
> >>   PCI_ACS_UF if p2p is supported.
> >>
> >> Did I get this right?  
> >   
> 
> I'm looking for sanity check on OS requirements. According to the PCIE spec,
> ACS itself is optional. However, Linux requires ACS capability. I want to
> make sure that we are satisfying the Linux requirements here.

This is like saying Linux requires ACPI, it's simply not true.  ACS is
a PCIe spec defined feature which gives us a standard mechanism of
determining if a device is DMA isolated within a PCIe topology.  For
those that do not implement the standard, we have numerous quirks
implementing platform specific mechanism to provide the equivalent
behavior.  Even this device isolation is not required by Linux, except
for the use case of userspace drivers, where we make the incredibly
reasonable requirement (IMO) that we cannot mix devices which are not
isolated from one another between kernel and user or one user and
another.
 
> I also agree that spec should be the guide. Maybe, a combination of spec+linux
> is the right answer.
> 
> > I'd suggest following the spec, not the code, that way you always have a
> > case for how you interpret the spec and the behavior of your hardware.
> > The code is an attempt to validate the device against the spec, but more
> > thorough implementations may follow.  REQ_ACS_FLAGS defines the set of
> > ACS capabilities we think are relevant to device isolation.  In
> > particular, UF seems like a key feature and our current test for it may
> > not be fully correct.  Note how RR and CR only specify p2p with other
> > root ports while UF requires transaction towards to the RC.  Section
> > 6.12.2 further defines Redirected Request Validation as a feature
> > within the context of RR and CR.  So rather than look at the code,
> > discuss section 6.12 with the hardware engineers and understand how it
> > behaves relative to each device and transaction type.  Implement the
> > capabilities that best match.  Attempting a minimal implementation
> > based on the current software interpretation may bite later, for
> > instance if we re-interpret how UF works.  Thanks,  
> 
> I read your reply multiple times. Here is what I got from it.
> 
> The summary below is for the root ports only.
> 
> - P2P on root ports is optional. 

And unless told otherwise (by ACS or ACS-equivalent quirks), Linux
assumes p2p is possible.

> - If P2P is there source validation, translation blocking, upstream 
> forwarding,
> p2p request redirection and p2p completion redirection ACS capabilities need 
> to be
> there for spec+linux compliance.

Again, this is not a Linux requirement.  If you want Linux to recognize
the isolation of devices downstream of the root port using standard
mechanism, this would be the way to do so.
 
> - If P2P is not there source validation, translation blocking and upstream 
> forwarding
> needs to be there for spec+linux compliance.

This is interpretation of a rather confusing section of the spec where
we currently take a fairly liberal approach.  The spec says RR must be
implemented by RPs that support p2p traffic _with_other_root_ports_.
That doesn't necessarily say anything about p2p "reflected" back
downstream, however enabling RR clearly states that p2p request must be
redirected upstream.  Thus supporting RR is clearly the safer choice
here for long term compliance if your device uses the RR behavior
already.  Supporting CR also falls out of that since RR necessitates
CR.  UF also references RR and CR behavior when enabled, so as per the
footnote in 6.12.1.1, it's not entirely clear that we can assume what
happens if a RP supports UF but not RR/CR.  The safe bet is to
implement each capability that matches your hardware behavior.

> - The code is relaxed to allow any combination of these today based on the ACS
> capability but it can change tomorrow. 

Yes, we somewhat assume that if a device supports ACS at all, it's
probably doing the "right thing".  I don't know that the current
behavior of assuming p2p is not possible when the capability is not
present is actually used by any existing hardware.  It seems
questionable on a strict reading of the spec.  Thanks,

Alex
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

RE: [PATCH] iommu/amd: flush IOTLB for specific domains only

[Nath, Arindam]

>-Original Message-
>From: Daniel Drake [mailto:dr...@endlessm.com]
>Sent: Thursday, March 30, 2017 7:15 PM
>To: Nath, Arindam
>Cc: j...@8bytes.org; Deucher, Alexander; Bridgman, John; amd-
>g...@lists.freedesktop.org; iommu@lists.linux-foundation.org; Suthikulpanit,
>Suravee; Linux Upstreaming Team
>Subject: Re: [PATCH] iommu/amd: flush IOTLB for specific domains only
>
>On Thu, Mar 30, 2017 at 12:23 AM, Nath, Arindam 
>wrote:
>> Daniel, did you get chance to test this patch?
>
>Not yet. Should we test it alone or alongside "PCI: Blacklist AMD
>Stoney GPU devices for ATS"?

Daniel, any luck with this patch?

Thanks,
Arindam

>
>Thanks
>Daniel
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [RFC] PCI ACS Flags Clarification

[Sinan Kaya]

Hi Alex,

Thank you very much for the detailed explanation.

On 4/4/2017 3:39 PM, Alex Williamson wrote:
> On Tue, 4 Apr 2017 14:47:58 -0400
> Sinan Kaya  wrote:
> 
[cut]

>> The requirement is to have an 
>> 1. ACS capability with PCI_ACS_SV if p2p is not supported
>> 2. ACS capability with PCI_ACS_SV|PCI_ACS_RR | PCI_ACS_CR |
>>   PCI_ACS_UF if p2p is supported.
>>
>> Did I get this right?
> 

I'm looking for sanity check on OS requirements. According to the PCIE spec,
ACS itself is optional. However, Linux requires ACS capability. I want to
make sure that we are satisfying the Linux requirements here.

I also agree that spec should be the guide. Maybe, a combination of spec+linux
is the right answer.

> I'd suggest following the spec, not the code, that way you always have a
> case for how you interpret the spec and the behavior of your hardware.
> The code is an attempt to validate the device against the spec, but more
> thorough implementations may follow.  REQ_ACS_FLAGS defines the set of
> ACS capabilities we think are relevant to device isolation.  In
> particular, UF seems like a key feature and our current test for it may
> not be fully correct.  Note how RR and CR only specify p2p with other
> root ports while UF requires transaction towards to the RC.  Section
> 6.12.2 further defines Redirected Request Validation as a feature
> within the context of RR and CR.  So rather than look at the code,
> discuss section 6.12 with the hardware engineers and understand how it
> behaves relative to each device and transaction type.  Implement the
> capabilities that best match.  Attempting a minimal implementation
> based on the current software interpretation may bite later, for
> instance if we re-interpret how UF works.  Thanks,

I read your reply multiple times. Here is what I got from it.

The summary below is for the root ports only.

- P2P on root ports is optional. 
- If P2P is there source validation, translation blocking, upstream forwarding,
p2p request redirection and p2p completion redirection ACS capabilities need to 
be
there for spec+linux compliance. 
- If P2P is not there source validation, translation blocking and upstream 
forwarding
needs to be there for spec+linux compliance.
- The code is relaxed to allow any combination of these today based on the ACS
capability but it can change tomorrow. 

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.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH V10 00/12] IOMMU probe deferral support

[Lorenzo Pieralisi]

On Tue, Apr 04, 2017 at 01:49:29PM +0100, Robin Murphy wrote:
> On 04/04/17 11:18, Sricharan R wrote:
> > This series calls the dma ops configuration for the devices
> > at a generic place so that it works for all busses.
> > The dma_configure_ops for a device is now called during
> > the device_attach callback just before the probe of the
> > bus/driver is called. Similarly dma_deconfigure is called during
> > device/driver_detach path.
> > 
> > pci_bus_add_devices(platform/amba)(_device_create/driver_register)
> >| |
> > pci_bus_add_device (device_add/driver_register)
> >| |
> > device_attach   device_initial_probe
> >| |
> > __device_attach_driver__device_attach_driver
> >|
> > driver_probe_device
> >|
> > really_probe
> >|
> > dma_configure
> > 
> > Similarly on the device/driver_unregister path __device_release_driver is
> > called which inturn calls dma_deconfigure.
> > 
> > Rebased the series against mainline 4.11-rc5. Applies and builds cleanly
> > against mainline and linux-next, iommu-next.
> >   
> > * Tested with platform and pci devices for probe deferral
> >   and reprobe on arm64 based platform.
> 
> Sricharan, thanks for keeping this going - I really think we're there now :)

FYI, I re-tested this series with ACPI and tried a merge with IORT
patches queued for 4.12 (via arm64) and there does not seem to be any
merge conflicts so it should be ready to go.

Thanks,
Lorenzo

> Joerg, I realise that at -rc5 time is getting on a bit already, but even
> the non-vintage parts of the series are pretty mature now so it would be
> nice to at least give it a spin in -next. If you don't quite share my
> confidence for landing it in 4.12, please consider it for early next
> cycle to get a full workout.
> 
> Thanks,
> Robin.
> 
> > Previous post of this series [8]. 
> > 
> > Please note that, i have kept the tested/acked tags intact from V8
> > because V9/10 were for more fixes that was added, so the original
> > tags that was given for the functional testing remains the same.
> > 
> >  [V10]
> >  * Rebased on top of 4.11-rc5.
> >  
> >  * Fixed coherent_dma_mask 64bit overflow issue [8]
> >for OF. The fix for OF was added as a separate
> >patch#6, since the issue is true even without probe deferral,
> >but gets reproduced with the probe deferral series.
> >Added Lorenzo's ACPI fix for coherent_dma_mask overflow
> >and the fix for dma_configure getting called more than
> >once for the same device.
> > 
> >  * Also fixed an build issue caught by kbuild robot for
> >m68k arch. The issue was dma_(de)configure was not
> >getting defined for !CONFIG_HAS_DMA, so fixed that as well.
> > 
> >  [V9]
> >  * Rebased on top of 4.11-rc1.
> > 
> >  * Merged Robin's fixes for legacy binding issue,
> >pci devices with no iommu-map property and deferencing
> >of_iommu_table after init.
> >  
> >  [V8]
> >  * Picked up all the acks and tested tags from Marek and
> >Hanjun for DT and ACPI patches respectively, since
> >no functional changes was done.
> > 
> >  * Addressed Minor comments Sinan and Bjorn.
> > 
> >  * Added Robin's fix for fixing the deferencing NULL for
> >of_iommu_table after init in patch #2.
> > 
> >  * Rebased it on top of linux-next
> > 
> >  [V7]
> >  * Updated the subject and commit log for patch #6 as per
> >comments from Lorenzo. No functional changes.
> > 
> >  [V6]
> >  * Fixed a bug in dma_configure function pointed out by
> >Robin.
> >  * Reordered the patches as per comments from Robin and
> >Lorenzo.
> >  * Added Tags.
> > 
> >  [V5]
> >  * Reworked the pci configuration code hanging outside and
> >pushed it to dma_configure as in PATCH#5,6,7.
> >Also added a couple of patches that Lorenzo provided for
> >correcting the Probe deferring mechanism in case of
> >ACPI devices from here [5].
> > 
> >  [V4]
> >  * Took the reworked patches [2] from Robin's branch and
> >rebased on top of Lorenzo's ACPI IORT ARM support series [3].
> > 
> >  * Added the patches for moving the dma ops configuration of
> >acpi based devices to probe time as well.
> >  [V3]
> >  * Removed the patch to split dma_masks/dma_ops configuration
> >separately based on review comments that both masks and ops are
> >required only during the device probe time.
> > 
> >  * Reworked the series based on Generic DT bindings series.
> > 
> >  * Added call to iommu's remove_device in the cleanup path for arm and
> >arm64.
> > 
> >  * Removed the notifier trick in arm64 to handle early device
> >registration.
> > 
> >  * Added reset of dma_ops in cleanup path for arm based on comments.
> > 
> >  * Fixed