Re: [patch 37/37] dmaengine: qcom_hidma: Cleanup MSI handling
On 11/26/2021 8:22 PM, Thomas Gleixner wrote: There is no reason to walk the MSI descriptors to retrieve the interrupt number for a device. Use msi_get_virq() instead. Signed-off-by: Thomas Gleixner Cc: Sinan Kaya Cc:dmaeng...@vger.kernel.org Acked-by: Sinan Kaya ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 1/4] ACPI/IORT: Check ATS capability in root complex nodes
On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: err = pci_for_each_dma_alias(to_pci_dev(dev), iort_pci_iommu_init, ); + + if (!err && !iort_pci_rc_supports_ats(node)) + dev->iommu_fwspec->flags |= IOMMU_FWSPEC_PCI_NO_ATS; Is it possible to remove !err check here. ATS supported is just a flag in IORT table. Does this decision have to rely on having a correct dma alias? ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH 3/4] iommu/arm-smmu-v3: Add support for PCI ATS
On 3/20/2019 1:36 PM, Jean-Philippe Brucker wrote: pos = pci_find_ext_capability(pdev, PCI_EXT_CAP_ID_ATS); + if (!pos) + return -ENOSYS; + You don't need this. pci_enable_ats() validates this via. if (!dev->ats_cap) return -EINVAL; ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/11/2019 2:15 PM, Raj, Ashok wrote: It seems rather odd we have to check for ATS version. I always assumed unspecified bits (Reserved) must be 0. We only check this if ATS is enabled, and this particular bit wasn't given away for another feature. Is it really required to check for ATS version before consuming this? Reading again, it looks like version check is not necessary since it is implied by the presence of this bit per this paragraph. Page Aligned Request – If Set, indicates the Untranslated Address is always aligned to a 4096 byte boundary. Setting this bit is recommended. This bit permits software to distinguish between implementations compatible with earlier version of this specification that permitted a requester to supply anything in bits [11:2]. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/8/2019 8:02 PM, sathyanarayanan kuppuswamy wrote: This means that you should probably have some kind of version check here. There is no version field in ATS v1.0 spec. Also, If I follow the history log in PCI spec, I think ATS if first added at v1.2. Please correct me if I am wrong. v1.2 was incorporated into PCIe spec at that time. However, the ATS spec is old and there could be some HW that could claim to be ATS compatible. I know AMD GPUs declare ATS capability. See this ECN https://composter.com.ua/documents/ats_r1.1_26Jan09.pdf You need to validate the version field from ATS capability header to be 1 before reading this register. See Table 5-1: ATS Extended Capability Header ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/7/2019 5:16 PM, sathyanarayanan kuppuswamy wrote: If I remember this right, aligned request is only supported on ATS v1.1 but not supported on v1.0. Its added in v1.1. This means that you should probably have some kind of version check here. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH v1 1/2] PCI: ATS: Add function to check ATS page aligned request status.
On 2/7/2019 1:41 PM, sathyanarayanan.kuppusw...@linux.intel.com wrote:
+ * As per PCI spec, If page aligned request bit is set, it indicates
+ * the untranslated address is always aligned to a 4096 byte boundary.
+ */
+int pci_ats_page_aligned(struct pci_dev *pdev)
+{
+ u16 cap;
+
+ if (!pdev->ats_cap)
+ return 0;
+
+ pci_read_config_word(pdev, pdev->ats_cap + PCI_ATS_CAP, );
If I remember this right, aligned request is only supported on ATS v1.1
but not supported on v1.0.
Can you please check the spec?
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3] PCI: Enable PASID when End-to-End TLP is supported by all bridges
On 9/26/2018 1:20 PM, Sinan Kaya wrote: This patch actually broke the integrated endpoints like you were mentioning. It was fixed by a follow up patch from nvidia guys. https://github.com/torvalds/linux/commit/9d27e39d309c93025ae6aa97236af15bef2a5f1f#diff-a7f0acd715e991df5dfb450d2b9abebc Turns out Felix works at AMD. Apologies for the name confusion. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V3] PCI: Enable PASID when End-to-End TLP is supported by all bridges
On 9/26/2018 12:22 PM, Raj, Ashok wrote: pcie_capability_read_dword(dev, PCI_EXP_DEVCAP2, ); + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP)) + return; Forgot to notice this.. I'm not sure if the same enforcement is required for devices that are RCIEP. The spec isn't clear about calling any excemption. Although it should be simple for devices to expose e2etlp support, but if they don't that should be ok, since there are nothing between itself and the root port. We are seeking help from our SIG reps, but thought I'll ask here as well if there are other opinions. This patch actually broke the integrated endpoints like you were mentioning. It was fixed by a follow up patch from nvidia guys. https://github.com/torvalds/linux/commit/9d27e39d309c93025ae6aa97236af15bef2a5f1f#diff-a7f0acd715e991df5dfb450d2b9abebc ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] ia64: fix barrier placement for write* / dma mapping
On 8/1/2018 12:29 AM, Christoph Hellwig wrote: I asked this question to Tony Luck before. If I remember right, his answer was: CPU guarantees outstanding writes to be flushed when a register write instruction is executed and an additional barrier instruction is not needed. That would be great. It still doesn't explain the barriers in the dma sync routines. Those have been there since the following commit in the history tree: Yeah, I'll let Tony confirm my understanding. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH] PCI: Do not enable PASID when End-to-End TLP is not supported
+iommu folks.
On 5/19/2018 12:52 PM, Sinan Kaya wrote:
> A PCIe endpoint carries the process address space identifier (PASID) in
> the TLP prefix as part of the memory read/write transaction. The address
> information in the TLP is relevant only for a given PASID context.
>
> A translation agent takes PASID value and the address information from the
> TLP to look up the physical address in the system.
>
> If a bridge drops the TLP prefix, the translation agent can resolve the
> address to an incorrect location and cause data corruption. Prevent
> this condition by requiring End-to-End TLP prefix to be supported on the
> entire data path between the endpoint and the root port.
>
> Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
> ---
> drivers/pci/ats.c | 16
> include/uapi/linux/pci_regs.h | 1 +
> 2 files changed, 17 insertions(+)
>
> diff --git a/drivers/pci/ats.c b/drivers/pci/ats.c
> index 89305b5..0bcded5 100644
> --- a/drivers/pci/ats.c
> +++ b/drivers/pci/ats.c
> @@ -265,7 +265,9 @@ EXPORT_SYMBOL_GPL(pci_reset_pri);
> int pci_enable_pasid(struct pci_dev *pdev, int features)
> {
> u16 control, supported;
> + struct pci_dev *bridge;
> int pos;
> + u32 cap;
>
> if (WARN_ON(pdev->pasid_enabled))
> return -EBUSY;
> @@ -274,6 +276,20 @@ int pci_enable_pasid(struct pci_dev *pdev, int features)
> if (!pos)
> return -EINVAL;
>
> + bridge = pci_upstream_bridge(pdev);
> + while (bridge) {
> + if (!pci_find_capability(bridge, PCI_CAP_ID_EXP))
> + return -EINVAL;
> +
> + if (pcie_capability_read_dword(bridge, PCI_EXP_DEVCAP2, ))
> + return -EINVAL;
> +
> + if (!(cap & PCI_EXP_DEVCAP2_E2ETLP))
> + return -EINVAL;
> +
> + bridge = pci_upstream_bridge(bridge);
> + }
> +
> pci_read_config_word(pdev, pos + PCI_PASID_CAP, );
> supported &= PCI_PASID_CAP_EXEC | PCI_PASID_CAP_PRIV;
>
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 103ba79..d91dea5 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -634,6 +634,7 @@
> #define PCI_EXP_DEVCAP2_OBFF_MASK 0x000c /* OBFF support mechanism */
> #define PCI_EXP_DEVCAP2_OBFF_MSG0x0004 /* New message signaling */
> #define PCI_EXP_DEVCAP2_OBFF_WAKE 0x0008 /* Re-use WAKE# for OBFF */
> +#define PCI_EXP_DEVCAP2_E2ETLP 0x0020 /* End-to-End TLP
> Prefix */
> #define PCI_EXP_DEVCTL2 40 /* Device Control 2 */
> #define PCI_EXP_DEVCTL2_COMP_TIMEOUT0x000f /* Completion Timeout
> Value */
> #define PCI_EXP_DEVCTL2_COMP_TMOUT_DIS 0x0010 /* Completion Timeout
> Disable */
>
--
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 v2 35/40] iommu/arm-smmu-v3: Add support for PCI ATS
On 5/11/2018 3:06 PM, Jean-Philippe Brucker wrote: > PCIe devices can implement their own TLB, named Address Translation Cache > (ATC). Enable Address Translation Service (ATS) for devices that support > it and send them invalidation requests whenever we invalidate the IOTLBs. > > Range calculation > - > > The invalidation packet itself is a bit awkward: range must be naturally > aligned, which means that the start address is a multiple of the range > size. In addition, the size must be a power of two number of 4k pages. We > have a few options to enforce this constraint: > > (1) Find the smallest naturally aligned region that covers the requested > range. This is simple to compute and only takes one ATC_INV, but it > will spill on lots of neighbouring ATC entries. > > (2) Align the start address to the region size (rounded up to a power of > two), and send a second invalidation for the next range of the same > size. Still not great, but reduces spilling. > > (3) Cover the range exactly with the smallest number of naturally aligned > regions. This would be interesting to implement but as for (2), > requires multiple ATC_INV. > > As I suspect ATC invalidation packets will be a very scarce resource, I'll > go with option (1) for now, and only send one big invalidation. We can > move to (2), which is both easier to read and more gentle with the ATC, > once we've observed on real systems that we can send multiple smaller > Invalidation Requests for roughly the same price as a single big one. > > Note that with io-pgtable, the unmap function is called for each page, so > this doesn't matter. The problem shows up when sharing page tables with > the MMU. > > Timeout > --- > > ATC invalidation is allowed to take up to 90 seconds, according to the > PCIe spec, so it is possible to hit the SMMU command queue timeout during > normal operations. > > Some SMMU implementations will raise a CERROR_ATC_INV_SYNC when a CMD_SYNC > fails because of an ATC invalidation. Some will just abort the CMD_SYNC. > Others might let CMD_SYNC complete and have an asynchronous IMPDEF > mechanism to record the error. When we receive a CERROR_ATC_INV_SYNC, we > could retry sending all ATC_INV since last successful CMD_SYNC. When a > CMD_SYNC fails without CERROR_ATC_INV_SYNC, we could retry sending *all* > commands since last successful CMD_SYNC. > > We cannot afford to wait 90 seconds in iommu_unmap, let alone MMU > notifiers. So we'd have to introduce a more clever system if this timeout > becomes a problem, like keeping hold of mappings and invalidating in the > background. Implementing safe delayed invalidations is a very complex > problem and deserves a series of its own. We'll assess whether more work > is needed to properly handle ATC invalidation timeouts once this code runs > on real hardware. > > Misc > > > I didn't put ATC and TLB invalidations in the same functions for three > reasons: > > * TLB invalidation by range is batched and committed with a single sync. > Batching ATC invalidation is inconvenient, endpoints limit the number of > inflight invalidations. We'd have to count the number of invalidations > queued and send a sync periodically. In addition, I suspect we always > need a sync between TLB and ATC invalidation for the same page. > > * Doing ATC invalidation outside tlb_inv_range also allows to send less > requests, since TLB invalidations are done per page or block, while ATC > invalidations target IOVA ranges. > > * TLB invalidation by context is performed when freeing the domain, at > which point there isn't any device attached anymore. > > Signed-off-by: Jean-Philippe Brucker <jean-philippe.bruc...@arm.com> Nothing specific about this patch but just a general observation. Last time I looked at the code, it seemed to require both ATS and PRI support from a given hardware. I think you can assume that for ATS 1.1 specification but ATS 1.0 specification allows a system to have ATS+PASID without PRI. QDF2400 is ATS 1.0 compatible as an example. Is this an assumption / my misinterpretation? -- 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: [RFC/RFT] Add noats flag to boot parameters
+Bjorn, On 5/3/2018 9:59 AM, Joerg Roedel wrote: > On Thu, May 03, 2018 at 09:46:34AM -0400, Sinan Kaya wrote: >> I also like the idea in general. >> Minor nit.. >> >> Shouldn't this be an iommu parameter rather than a PCI kernel command line >> parameter? >> We now have an iommu.passthrough argument that prevents page translation. >> >> Doesn't this fit into the same category especially when it is the IOMMU >> drivers that >> call ATS functions for enablement not the PCI drivers. > > ATS is a bit of a grey area between PCI and IOMMU, but since ATS is > PCI-specific and the code to enable/disable it is in PCI as well, I > think the parameter makes sense for PCI too. > OK. Bjorn was interested in having a command line driven feature enables in driver/pci directory with bitmasks for each optional PCI spec capability rather than noXYZ feature. This would allow us to troubleshoot code breakage as well as the platform bring up to turn off all optional features. Sounds like this would be a good match for that work. > > Joerg > > -- 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: [RFC/RFT] Add noats flag to boot parameters
On 5/3/2018 9:35 AM, Joerg Roedel wrote: > On Sun, Apr 29, 2018 at 09:16:48PM +0300, Gil Kupfer wrote: >> This patch adds noats option to the pci boot parameter. >> When noats is selected, all ATS related functions fail immediately and >> the IOMMU is configured to not use device-iotlb. >> >> Any function that checks for ATS capabilities directly against the >> devices should also check this flag. (Currently, such functions exist >> only in IOMMU drivers, and they are covered by this patch.) >> >> The motivation behind this patch is the existence of malicious devices. >> Lots of research has been done about how to utilitize the IOMMU as a >> protection from such devices. When ATS is supported, any I/O device can >> access any physical access by faking device-IOTLB entries. >> Adding the ability to ignore these entries lets sysadmins enhance system >> security. >> >> Signed-off-by: Gil Kupfer <gil...@cs.technion.ac.il> > > This has also been on my list, thanks for doing that. > > Acked-by: Joerg Roedel <jroe...@suse.de> > I also like the idea in general. Minor nit.. Shouldn't this be an iommu parameter rather than a PCI kernel command line parameter? We now have an iommu.passthrough argument that prevents page translation. Doesn't this fit into the same category especially when it is the IOMMU drivers that call ATS functions for enablement not the PCI drivers. -- 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 03/37] iommu/sva: Manage process address spaces
On 4/24/2018 5:33 AM, Jean-Philippe Brucker wrote: >> Please return pasid when you find an io_mm that is already bound. Something >> like >> *pasid = io_mm->pasid should do the work here when bond is true. > Right. I think we should also keep returning 0, not switch to -EEXIST or > similar. So in next version a driver can call bind(devX, mmY) multiple > times, but the first unbind() removes the bond. If we are going to allow multiple binds, then the last unbind should remove the bond rather than the first one via reference counting. -- 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 03/37] iommu/sva: Manage process address spaces
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote:
> /**
> * iommu_sva_device_init() - Initialize Shared Virtual Addressing for a
> device
> * @dev: the device
> @@ -129,7 +439,10 @@ EXPORT_SYMBOL_GPL(iommu_sva_device_shutdown);
> int iommu_sva_bind_device(struct device *dev, struct mm_struct *mm, int
> *pasid,
> unsigned long flags, void *drvdata)
> {
> + int i, ret;
> + struct io_mm *io_mm = NULL;
> struct iommu_domain *domain;
> + struct iommu_bond *bond = NULL, *tmp;
> struct iommu_param *dev_param = dev->iommu_param;
>
> domain = iommu_get_domain_for_dev(dev);
> @@ -145,7 +458,42 @@ int iommu_sva_bind_device(struct device *dev, struct
> mm_struct *mm, int *pasid,
> if (flags != (IOMMU_SVA_FEAT_PASID | IOMMU_SVA_FEAT_IOPF))
> return -EINVAL;
>
> - return -ENOSYS; /* TODO */
> + /* If an io_mm already exists, use it */
> + spin_lock(_sva_lock);
> + idr_for_each_entry(_pasid_idr, io_mm, i) {
> + if (io_mm->mm != mm || !io_mm_get_locked(io_mm))
> + continue;
> +
> + /* Is it already bound to this device? */
> + list_for_each_entry(tmp, _mm->devices, mm_head) {
> + if (tmp->dev != dev)
> + continue;
> +
> + bond = tmp;
> + refcount_inc(>refs);
> + io_mm_put_locked(io_mm);
> + break;
> + }
> + break;
> + }
> + spin_unlock(_sva_lock);
> +
> + if (bond)
Please return pasid when you find an io_mm that is already bound. Something like
*pasid = io_mm->pasid should do the work here when bond is true.
> + return 0;
--
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 V2] drm/amdgpu: limit DMA size to PAGE_SIZE for scatter-gather buffers
On 4/11/2018 8:03 AM, Robin Murphy wrote:
> On 10/04/18 21:59, Sinan Kaya wrote:
>> Code is expecing to observe the same number of buffers returned from
>> dma_map_sg() function compared to sg_alloc_table_from_pages(). This
>> doesn't hold true universally especially for systems with IOMMU.
>
> So why not fix said code? It's clearly not a real hardware limitation, and
> the map_sg() APIs have potentially returned fewer than nents since forever,
> so there's really no excuse.
Sure, I'll take a better fix if there is one.
>
>> IOMMU driver tries to combine buffers into a single DMA address as much
>> as it can. The right thing is to tell the DMA layer how much combining
>> IOMMU can do.
>
> Disagree; this is a dodgy hack, since you'll now end up passing scatterlists
> into dma_map_sg() which already violate max_seg_size to begin with, and I
> think a conscientious DMA API implementation would be at rights to fail the
> mapping for that reason (I know arm64 happens not to, but that was a
> deliberate design decision to make my life easier at the time).
>
> As a short-term fix, at least do something like what i915 does and constrain
> the table allocation to the desired segment size as well, so things remain
> self-consistent. But still never claim that faking a hardware constraint as a
> workaround for a driver shortcoming is "the right thing to do" ;)
You are asking for something like this from here, right?
https://elixir.bootlin.com/linux/v4.16.1/source/drivers/gpu/drm/i915/i915_gem_dmabuf.c#L58
ret = sg_alloc_table(st, obj->mm.pages->nents, GFP_KERNEL);
if (ret)
goto err_free;
src = obj->mm.pages->sgl;
dst = st->sgl;
for (i = 0; i < obj->mm.pages->nents; i++) {
sg_set_page(dst, sg_page(src), src->length, 0);
dst = sg_next(dst);
src = sg_next(src);
}
This seems to allocate the scatter gather list and fill it in manually before
passing it
to dma_map_sg(). I'll give it a try.
Just double checking.
>
> Robin.
>
>> Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
--
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 03/37] iommu/sva: Manage process address spaces
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote:
> +static void io_mm_detach_all_locked(struct iommu_bond *bond)
> +{
> + while (!io_mm_detach_locked(bond));
> +}
> +
I don't remember if I mentioned this before or not but I think this loop
needs a little bit relaxation with yield and maybe an informational message
with might help if wait exceeds some time.
--
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] dma-mapping: move dma configuration to bus infrastructure
On 3/12/2018 11:24 AM, Nipun Gupta wrote:
> + if (dma_dev->of_node) {
> + ret = of_dma_configure(dev, dma_dev->of_node);
> + } else if (has_acpi_companion(dma_dev)) {
> + attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode));
> + if (attr != DEV_DMA_NOT_SUPPORTED)
> + ret = acpi_dma_configure(dev, attr);
> + }
> +
> + pci_put_host_bridge_device(bridge);
> +
> + return ret;
> +}
> +
> +void pci_dma_deconfigure(struct device *dev)
> +{
> + of_dma_deconfigure(dev);
> + acpi_dma_deconfigure(dev);
> +}
Isn't this one or the other one but not both?
Something like:
if (dev->of_node)
of_dma_deconfigure(dev);
else
acpi_dma_deconfigure(dev);
should work.
--
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 07/37] iommu: Add a page fault handler
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote: > +static struct workqueue_struct *iommu_fault_queue; Is there anyway we can make this fault queue per struct device? Since this is common code, I think it needs some care. -- 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 07/37] iommu: Add a page fault handler
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote:
> +static int iommu_queue_fault(struct iommu_domain *domain, struct device *dev,
> + struct iommu_fault_event *evt)
> +{
> + struct iommu_fault_group *group;
> + struct iommu_fault_context *fault, *next;
> +
> + if (!iommu_fault_queue)
> + return -ENOSYS;
> +
> + if (!evt->last_req) {
> + fault = kzalloc(sizeof(*fault), GFP_KERNEL);
> + if (!fault)
> + return -ENOMEM;
> +
> + fault->evt = *evt;
> + fault->dev = dev;
> +
> + /* Non-last request of a group. Postpone until the last one */
> + spin_lock(_partial_faults_lock);
> + list_add_tail(>head, _partial_faults);
> + spin_unlock(_partial_faults_lock);
> +
> + return IOMMU_PAGE_RESP_HANDLED;
> + }
> +
> + group = kzalloc(sizeof(*group), GFP_KERNEL);
> + if (!group)
> + return -ENOMEM;
Release the requests in iommu_partial_faults here.
--
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: RFC on Kdump and PCIe on ARM64
On 3/2/2018 1:02 PM, Will Deacon wrote: >> How about the points that Baoquan highlighted in his email regarding the >> solution from AMD and X86? > Which specific points do you think this proposal doesn't address? > No specific concerns at this moment. >> I have not read the entire thread but, is this just a matter of following >> what Bjorn recommended or there is more to it? > I'm trying to say how I think Bjorn's idea can be implemented for SMMUv3. > I basically want to avoid a situation where the SMMU driver tries to walk > the in-memory data structures left by a previous kernel and infer the > setup from that. Let me know if you have something to test. We have a test case with the crash. -- 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: RFC on Kdump and PCIe on ARM64
On 3/1/2018 7:03 PM, Bjorn Helgaas wrote: >> 3. The last one is adapter gets into fuzzy state due to not coming >> out of clean state in the second time init and being rejected by >> SMMUv3 multiple times. >> >> [ 16.093441] pci :01:00.0: aer_status: 0x0004, aer_mask: 0x >> [ 16.099356] pci :01:00.0: Malformed TLP >> [ 16.103522] pci :01:00.0: aer_layer=Transaction Layer, >> aer_agent=Receiver ID >> [ 16.110900] pci :01:00.0: aer_uncor_severity: 0x00062011 >> [ 16.116543] pci :01:00.0: TLP Header: 0a00a000 8100 01010100 >> > I'm not clear on this. I don't remember what an IOMMU fault looks > like to an Endpoint. Are you saying that if an Endpoint sees too many > of those faults, it gets into this "fuzzy state" (whatever that is :))? > Is this a hardware defect? Do we care (this is a kdump kernel, after > all)? If we do care, can we fix the device by resetting it? fuzzy=funky=funny=wierd Regardless of what we do in the IOMMU driver, I think we still have to reset the endpoint in order to have a clean initialization. I'm not sure if all endpoint drivers can recover an adapter from a live state. I wasn't expecting to see a Malformed TLP error. I was guessing that this was caused by SMMU giving a CA or UR to the endpoint or having a live adapter in the middle of driver initialization. I think we do care about the adapter coming up properly otherwise how would you collect the dumps from the system? I was expecting to come through the network interface and download it from the target. That's why, I was suggesting FLR/PM reset etc. when we know that we are booting a kdump kernel. -- 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: RFC on Kdump and PCIe on ARM64
Hi Will, On 3/2/2018 5:30 AM, Will Deacon wrote: >> Do you really have to reset the IOMMU? Can you just give it new page >> tables that start out with all IOVAs from all devices being invalid, >> then add valid mappings as drivers need them (presumably after the >> driver has done whatever it needs to so the device stops using the old >> DMA addresses)? > We already have the option to do that via the command line using the > disable_bypass option, so it just sounds like we need to take this into > account when resetting the SMMU to take care that GBPA is configured so > that transactions are terminated when SMMUEN=0. How about the points that Baoquan highlighted in his email regarding the solution from AMD and X86? I have not read the entire thread but, is this just a matter of following what Bjorn recommended or there is more to it? 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: RFC on Kdump and PCIe on ARM64
On 3/1/2018 2:05 PM, Bjorn Helgaas wrote: > [+cc Joerg, David, iommu list] > > On Thu, Mar 01, 2018 at 12:44:26PM -0500, Sinan Kaya wrote: >> Hi, >> >> We are seeing IOMMU faults when booting the kdump kernel on ARM64. >> >> [7.220162] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x02 received: >> [7.226123] arm-smmu-v3 arm-smmu-v3.0.auto:0x0102 >> [7.232023] arm-smmu-v3 arm-smmu-v3.0.auto:0x >> [7.237925] arm-smmu-v3 arm-smmu-v3.0.auto:0x >> [7.243827] arm-smmu-v3 arm-smmu-v3.0.auto:0x >> >> This is Nate's interpretation of the fault: >> >> "The PCI device is sending transactions just after the SMMU was >> reset/reinitialized which is problematic because the device has not >> yet been added to the SMMU and thus should not be doing *any* DMA. >> DMA from the PCI devices should be quiesced prior to starting the >> crashdump kernel or you risk overwriting portions of memory you >> meant to preserve. In this case the SMMU was actually doing you a >> favor by blocking these errant DMA operations!!" >> >> I think this makes sense especially for the IOMMU enabled case on >> the host where an IOVA can overlap with the region of memory kdump >> reserved for itself. >> >> Apparently, there has been similar concerns in the past. >> >> https://www.fujitsu.com/jp/documents/products/software/os/linux/catalog/LinuxConJapan2013-Indoh.pdf >> >> and was not addressed globally due to IOMMU+PCI driver ordering >> issues and bugs in HW due to hot reset. >> >> https://lkml.org/lkml/2012/8/3/160 >> >> Hot reset as mentioned is destructive and may not be the best >> implementation choice. However, most of the modern endpoints >> support PCIE function level reset. >> >> One other solution is for SMMUv3 driver to reserve the kdump used >> IOVA addresses. >> >> Another solution is for the SMMUv3 driver to disable PCIe devices >> behind the SMMU if it see SMMU is already enabled. > > What problem are you trying to solve? If the IOMMU is blocking DMA > after the kdump kernel starts up, that sounds like the desired > behavior. > Three issues: 1. I'm seeing a flood of SMMUv3 faults due to adapter using addresses from the previous kernel. This might be OK. 2. When the SMMUv3 driver sees that it is enabled, it resets itself and configures it one more time. [7.018304] arm-smmu-v3 arm-smmu-v3.0.auto: ias 44-bit, oas 44-bit (features 0x1fef) [7.026379] arm-smmu-v3 arm-smmu-v3.0.auto: SMMU currently enabled! Resetting... >From the moment IOMMU is disabled to the point where IOMMU get enabled again, there is a potential for the PCIE device to corrupt the kdump kernel memory as the bus master and memory enable bits are left enabled. [0.00] crashkernel reserved: 0x7fe0 - 0xffe0 (2048 MB) This region happens to overlap with the IOVA addresses that SMMUv3 driver on the main kernel is allocating. IOVA addresses start from 0x and get decremented on each allocation. 3. The last one is adapter gets into fuzzy state due to not coming out of clean state in the second time init and being rejected by SMMUv3 multiple times. [ 16.093441] pci :01:00.0: aer_status: 0x0004, aer_mask: 0x0000 [ 16.099356] pci :01:00.0: Malformed TLP [ 16.103522] pci :01:00.0: aer_layer=Transaction Layer, aer_agent=Receiver ID [ 16.110900] pci :01:00.0: aer_uncor_severity: 0x00062011 [ 16.116543] pci :01:00.0: TLP Header: 0a00a000 8100 01010100 -- 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 02/37] iommu/sva: Bind process address spaces to devices
On 2/12/2018 1:33 PM, Jean-Philippe Brucker wrote:
> +int iommu_sva_unbind_group(struct iommu_group *group, int pasid)
> +{
> + struct group_device *device;
> +
> + mutex_lock(>mutex);
> + list_for_each_entry(device, >devices, list)
> + iommu_sva_unbind_device(device->dev, pasid);
> + mutex_unlock(>mutex);
> +
> + return 0;
> +}
I think we should handle the errors returned by iommu_sva_unbind_device() here
or at least print a warning if we want to still continue unbinding.
--
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 37/37] vfio: Add support for Shared Virtual Addressing
+ kfree(vfio_mm);
> + }
> + }
> +
> +out_put_mm:
> + mutex_unlock(>lock);
> + mmput(mm);
> +
> + return ret;
> +}
> +
> +static long vfio_iommu_type1_unbind_process(struct vfio_iommu *iommu,
> + void __user *arg,
> + struct vfio_iommu_type1_bind *bind)
> +{
> + int ret = -EINVAL;
> + unsigned long minsz;
> + struct mm_struct *mm;
> + struct vfio_mm *vfio_mm;
> + struct vfio_iommu_type1_bind_process params;
> +
> + minsz = sizeof(*bind) + sizeof(params);
> + if (bind->argsz < minsz)
> + return -EINVAL;
> +
> + arg += sizeof(*bind);
> + if (copy_from_user(, arg, sizeof(params)))
> + return -EFAULT;
> +
> + if (params.flags & ~VFIO_IOMMU_BIND_PID)
> + return -EINVAL;
> +
> + /*
> + * We can't simply unbind a foreign process by PASID, because the
> + * process might have died and the PASID might have been reallocated to
> + * another process. Instead we need to fetch that process mm by PID
> + * again to make sure we remove the right vfio_mm. In addition, holding
> + * the mm guarantees that mm_users isn't dropped while we unbind and the
> + * exit_mm handler doesn't fire. While not strictly necessary, not
> + * having to care about that race simplifies everyone's life.
> + */
> + if (params.flags & VFIO_IOMMU_BIND_PID) {
> + mm = vfio_iommu_get_mm_by_vpid(params.pid);
> + if (IS_ERR(mm))
> + return PTR_ERR(mm);
> + } else {
> + mm = get_task_mm(current);
> + if (!mm)
> + return -EINVAL;
> + }
> +
I think you can merge mm failure in both states.
> + ret = -ESRCH;
> + mutex_lock(>lock);
> + list_for_each_entry(vfio_mm, >mm_list, next) {
> + if (vfio_mm->mm != mm)
> + continue;
> +
these loops look wierd
1. for loops + break
2. for loops + goto
how about closing the for loop here. and then return here if not vfio_mm
not found.
> + vfio_iommu_unbind(iommu, vfio_mm);
> + list_del(_mm->next);
> + kfree(vfio_mm);
> + ret = 0;
> + break;
> + }
> + mutex_unlock(>lock);
> + mmput(mm);
> +
> + return ret;
> +}
> +
--
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: [RFCv2 PATCH 05/36] iommu/process: Bind and unbind process to and from devices
Hi Jean-Philippe,
On 10/6/2017 9:31 AM, Jean-Philippe Brucker wrote:
> /**
> + * iommu_process_bind_device - Bind a process address space to a device
> + * @dev: the device
> + * @task: the process to bind
> + * @pasid: valid address where the PASID will be stored
> + * @flags: bond properties (IOMMU_PROCESS_BIND_*)
> + *
> + * Create a bond between device and task, allowing the device to access the
> + * process address space using the returned PASID.
> + *
> + * On success, 0 is returned and @pasid contains a valid ID. Otherwise, an
> error
> + * is returned.
> + */
> +int iommu_process_bind_device(struct device *dev, struct task_struct *task,
> + int *pasid, int flags)
This API doesn't play nice with endpoint device drivers that have PASID
limitations.
The AMD driver seems to have PASID limitations per product that are not being
advertised in the PCI capability.
device_iommu_pasid_init()
{
pasid_limit = min_t(unsigned int,
(unsigned int)(1 << kfd->device_info->max_pasid_bits),
iommu_info.max_pasids);
/*
* last pasid is used for kernel queues doorbells
* in the future the last pasid might be used for a kernel thread.
*/
pasid_limit = min_t(unsigned int,
pasid_limit,
kfd->doorbell_process_limit - 1);
}
kfd->device_info->max_pasid_bits seems to contain per device limitations.
Would you be willing to extend the API so that the requester can impose some
limit
on the PASID value that is getting allocated.
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 V4 11/26] iommu/amd: deprecate pci_get_bus_and_slot()
On 1/4/2018 11:28 AM, Gary R Hook wrote: > On 01/04/2018 06:25 AM, Sinan Kaya wrote: >> On 12/19/2017 12:37 AM, Sinan Kaya wrote: >>> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as >>> where a PCI device is present. This restricts the device drivers to be >>> reused for other domain numbers. >>> >>> Getting ready to remove pci_get_bus_and_slot() function in favor of >>> pci_get_domain_bus_and_slot(). >>> >>> Hard-code the domain number as 0 for the AMD IOMMU driver. > > > >> >> Any comments from the IOMMU people? >> > > pci_get_bus_and_slot() appears to (now) be a convenience function that wraps > pci_get_domain_bus_and_slot() while using a 0 for the domain value. Exactly > what you are doing here, albeit in a more overt way. > > How is this patch advantageous? Seems to me that if other domains need to be > enabled, that driver could be changed if and when that requirement arises. > > But perhaps I'm missing a nuance here. > > The benefit of the change was discussed here: https://lkml.org/lkml/2017/12/19/349 I hope it helps. -- 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 V4 11/26] iommu/amd: deprecate pci_get_bus_and_slot()
On 12/19/2017 12:37 AM, Sinan Kaya wrote:
> pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
> where a PCI device is present. This restricts the device drivers to be
> reused for other domain numbers.
>
> Getting ready to remove pci_get_bus_and_slot() function in favor of
> pci_get_domain_bus_and_slot().
>
> Hard-code the domain number as 0 for the AMD IOMMU driver.
>
> Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
> ---
> drivers/iommu/amd_iommu.c | 3 ++-
> drivers/iommu/amd_iommu_init.c | 9 +
> drivers/iommu/amd_iommu_v2.c | 3 ++-
> 3 files changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
> index 7d5eb00..821547b 100644
> --- a/drivers/iommu/amd_iommu.c
> +++ b/drivers/iommu/amd_iommu.c
> @@ -527,7 +527,8 @@ static void amd_iommu_report_page_fault(u16 devid, u16
> domain_id,
> struct iommu_dev_data *dev_data = NULL;
> struct pci_dev *pdev;
>
> - pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
> + pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
> +devid & 0xff);
> if (pdev)
> dev_data = get_dev_data(>dev);
>
> diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
> index 6fe2d03..4e4a615 100644
> --- a/drivers/iommu/amd_iommu_init.c
> +++ b/drivers/iommu/amd_iommu_init.c
> @@ -1697,8 +1697,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
> u32 range, misc, low, high;
> int ret;
>
> - iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
> - iommu->devid & 0xff);
> + iommu->dev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(iommu->devid),
> + iommu->devid & 0xff);
> if (!iommu->dev)
> return -ENODEV;
>
> @@ -1764,8 +1764,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
> if (is_rd890_iommu(iommu->dev)) {
> int i, j;
>
> - iommu->root_pdev = pci_get_bus_and_slot(iommu->dev->bus->number,
> - PCI_DEVFN(0, 0));
> + iommu->root_pdev =
> + pci_get_domain_bus_and_slot(0, iommu->dev->bus->number,
> + PCI_DEVFN(0, 0));
>
> /*
>* Some rd890 systems may not be fully reconfigured by the
> diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
> index 7d94e1d..8696382 100644
> --- a/drivers/iommu/amd_iommu_v2.c
> +++ b/drivers/iommu/amd_iommu_v2.c
> @@ -564,7 +564,8 @@ static int ppr_notifier(struct notifier_block *nb,
> unsigned long e, void *data)
> finish = (iommu_fault->tag >> 9) & 1;
>
> devid = iommu_fault->device_id;
> - pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
> + pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
> +devid & 0xff);
> if (!pdev)
> return -ENODEV;
> dev_data = get_dev_data(>dev);
>
Any comments from the IOMMU people?
--
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
[PATCH V4 11/26] iommu/amd: deprecate pci_get_bus_and_slot()
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Getting ready to remove pci_get_bus_and_slot() function in favor of
pci_get_domain_bus_and_slot().
Hard-code the domain number as 0 for the AMD IOMMU driver.
Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
---
drivers/iommu/amd_iommu.c | 3 ++-
drivers/iommu/amd_iommu_init.c | 9 +
drivers/iommu/amd_iommu_v2.c | 3 ++-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 7d5eb00..821547b 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -527,7 +527,8 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
struct iommu_dev_data *dev_data = NULL;
struct pci_dev *pdev;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (pdev)
dev_data = get_dev_data(>dev);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6fe2d03..4e4a615 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1697,8 +1697,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
u32 range, misc, low, high;
int ret;
- iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
- iommu->devid & 0xff);
+ iommu->dev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(iommu->devid),
+iommu->devid & 0xff);
if (!iommu->dev)
return -ENODEV;
@@ -1764,8 +1764,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
if (is_rd890_iommu(iommu->dev)) {
int i, j;
- iommu->root_pdev = pci_get_bus_and_slot(iommu->dev->bus->number,
- PCI_DEVFN(0, 0));
+ iommu->root_pdev =
+ pci_get_domain_bus_and_slot(0, iommu->dev->bus->number,
+ PCI_DEVFN(0, 0));
/*
* Some rd890 systems may not be fully reconfigured by the
diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 7d94e1d..8696382 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -564,7 +564,8 @@ static int ppr_notifier(struct notifier_block *nb, unsigned
long e, void *data)
finish = (iommu_fault->tag >> 9) & 1;
devid = iommu_fault->device_id;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (!pdev)
return -ENODEV;
dev_data = get_dev_data(>dev);
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V3 12/29] iommu/amd: deprecate pci_get_bus_and_slot()
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Getting ready to remove pci_get_bus_and_slot() function in favor of
pci_get_domain_bus_and_slot().
Hard-code the domain number as 0 for the AMD IOMMU driver.
Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
---
drivers/iommu/amd_iommu.c | 3 ++-
drivers/iommu/amd_iommu_init.c | 9 +
drivers/iommu/amd_iommu_v2.c | 3 ++-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8e8874d..2c3452f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -528,7 +528,8 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
struct iommu_dev_data *dev_data = NULL;
struct pci_dev *pdev;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (pdev)
dev_data = get_dev_data(>dev);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6fe2d03..4e4a615 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1697,8 +1697,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
u32 range, misc, low, high;
int ret;
- iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
- iommu->devid & 0xff);
+ iommu->dev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(iommu->devid),
+iommu->devid & 0xff);
if (!iommu->dev)
return -ENODEV;
@@ -1764,8 +1764,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
if (is_rd890_iommu(iommu->dev)) {
int i, j;
- iommu->root_pdev = pci_get_bus_and_slot(iommu->dev->bus->number,
- PCI_DEVFN(0, 0));
+ iommu->root_pdev =
+ pci_get_domain_bus_and_slot(0, iommu->dev->bus->number,
+ PCI_DEVFN(0, 0));
/*
* Some rd890 systems may not be fully reconfigured by the
diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 7d94e1d..8696382 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -564,7 +564,8 @@ static int ppr_notifier(struct notifier_block *nb, unsigned
long e, void *data)
finish = (iommu_fault->tag >> 9) & 1;
devid = iommu_fault->device_id;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (!pdev)
return -ENODEV;
dev_data = get_dev_data(>dev);
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH V2 12/29] iommu/amd: deprecate pci_get_bus_and_slot()
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Getting ready to remove pci_get_bus_and_slot() function in favor of
pci_get_domain_bus_and_slot().
Hard-code the domain number as 0 for the AMD IOMMU driver.
Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
---
drivers/iommu/amd_iommu.c | 3 ++-
drivers/iommu/amd_iommu_init.c | 9 +
drivers/iommu/amd_iommu_v2.c | 3 ++-
3 files changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8e8874d..2c3452f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -528,7 +528,8 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
struct iommu_dev_data *dev_data = NULL;
struct pci_dev *pdev;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (pdev)
dev_data = get_dev_data(>dev);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6fe2d03..4e4a615 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1697,8 +1697,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
u32 range, misc, low, high;
int ret;
- iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
- iommu->devid & 0xff);
+ iommu->dev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(iommu->devid),
+iommu->devid & 0xff);
if (!iommu->dev)
return -ENODEV;
@@ -1764,8 +1764,9 @@ static int iommu_init_pci(struct amd_iommu *iommu)
if (is_rd890_iommu(iommu->dev)) {
int i, j;
- iommu->root_pdev = pci_get_bus_and_slot(iommu->dev->bus->number,
- PCI_DEVFN(0, 0));
+ iommu->root_pdev =
+ pci_get_domain_bus_and_slot(0, iommu->dev->bus->number,
+ PCI_DEVFN(0, 0));
/*
* Some rd890 systems may not be fully reconfigured by the
diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 7d94e1d..8696382 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -564,7 +564,8 @@ static int ppr_notifier(struct notifier_block *nb, unsigned
long e, void *data)
finish = (iommu_fault->tag >> 9) & 1;
devid = iommu_fault->device_id;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (!pdev)
return -ENODEV;
dev_data = get_dev_data(>dev);
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 13/30] iommu/amd: deprecate pci_get_bus_and_slot()
pci_get_bus_and_slot() is restrictive such that it assumes domain=0 as
where a PCI device is present. This restricts the device drivers to be
reused for other domain numbers.
Use pci_get_domain_bus_and_slot() with a domain number of 0 where we can't
extract the domain number. Other places, use the actual domain number from
the device.
Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
---
drivers/iommu/amd_iommu.c | 3 ++-
drivers/iommu/amd_iommu_init.c | 10 ++
drivers/iommu/amd_iommu_v2.c | 3 ++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/iommu/amd_iommu.c b/drivers/iommu/amd_iommu.c
index 8e8874d..2c3452f 100644
--- a/drivers/iommu/amd_iommu.c
+++ b/drivers/iommu/amd_iommu.c
@@ -528,7 +528,8 @@ static void amd_iommu_report_page_fault(u16 devid, u16
domain_id,
struct iommu_dev_data *dev_data = NULL;
struct pci_dev *pdev;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (pdev)
dev_data = get_dev_data(>dev);
diff --git a/drivers/iommu/amd_iommu_init.c b/drivers/iommu/amd_iommu_init.c
index 6fe2d03..8d159ff 100644
--- a/drivers/iommu/amd_iommu_init.c
+++ b/drivers/iommu/amd_iommu_init.c
@@ -1697,8 +1697,8 @@ static int iommu_init_pci(struct amd_iommu *iommu)
u32 range, misc, low, high;
int ret;
- iommu->dev = pci_get_bus_and_slot(PCI_BUS_NUM(iommu->devid),
- iommu->devid & 0xff);
+ iommu->dev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(iommu->devid),
+iommu->devid & 0xff);
if (!iommu->dev)
return -ENODEV;
@@ -1763,9 +1763,11 @@ static int iommu_init_pci(struct amd_iommu *iommu)
if (is_rd890_iommu(iommu->dev)) {
int i, j;
+ u16 busnr = iommu->dev->bus->number;
+ u16 devfn = PCI_DEVFN(0, 0);
- iommu->root_pdev = pci_get_bus_and_slot(iommu->dev->bus->number,
- PCI_DEVFN(0, 0));
+ iommu->root_pdev = pci_get_domain_bus_and_slot(0, busnr,
+ devfn);
/*
* Some rd890 systems may not be fully reconfigured by the
diff --git a/drivers/iommu/amd_iommu_v2.c b/drivers/iommu/amd_iommu_v2.c
index 7d94e1d..8696382 100644
--- a/drivers/iommu/amd_iommu_v2.c
+++ b/drivers/iommu/amd_iommu_v2.c
@@ -564,7 +564,8 @@ static int ppr_notifier(struct notifier_block *nb, unsigned
long e, void *data)
finish = (iommu_fault->tag >> 9) & 1;
devid = iommu_fault->device_id;
- pdev = pci_get_bus_and_slot(PCI_BUS_NUM(devid), devid & 0xff);
+ pdev = pci_get_domain_bus_and_slot(0, PCI_BUS_NUM(devid),
+ devid & 0xff);
if (!pdev)
return -ENODEV;
dev_data = get_dev_data(>dev);
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [RFCv2 PATCH 05/36] iommu/process: Bind and unbind process to and from devices
Just some improvement suggestions.
On 10/6/2017 9:31 AM, Jean-Philippe Brucker wrote:
> + spin_lock(_process_lock);
> + idr_for_each_entry(_process_idr, process, i) {
> + if (process->pid != pid)
> + continue;
if you see this construct a lot, this could be a for_each_iommu_process.
> +
> + if (!iommu_process_get_locked(process)) {
> + /* Process is defunct, create a new one */
> + process = NULL;
> + break;
> + }
> +
> + /* Great, is it also bound to this domain? */
> + list_for_each_entry(cur_context, >domains,
> + process_head) {
> + if (cur_context->domain != domain)
> + continue;
if you see this construct a lot, this could be a for_each_iommu_process_domain.
> +
> + context = cur_context;
> + *pasid = process->pasid;
> +
> + /* Splendid, tell the driver and increase the ref */
> + err = iommu_process_attach_locked(context, dev);
> + if (err)
> + iommu_process_put_locked(process);
> +
> + break;
> + }
> + break;
> + }
> + spin_unlock(_process_lock);
> + put_pid(pid);
> +
> + if (context)
> + return err;
I think you should make the section above a independent function and return
here when the
context is found.
--
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: [RFCv2 PATCH 01/36] iommu: Keep track of processes and PASIDs
few nits below.
> +/*
> + * Allocate a iommu_process structure for the given task.
> + *
> + * Ideally we shouldn't need the domain parameter, since iommu_process is
> + * system-wide, but we use it to retrieve the driver's allocation ops and a
> + * PASID range.
> + */
> +static struct iommu_process *
> +iommu_process_alloc(struct iommu_domain *domain, struct task_struct *task)
> +{
> + int err;
> + int pasid;
> + struct iommu_process *process;
> +
> + if (WARN_ON(!domain->ops->process_alloc || !domain->ops->process_free))
> + return ERR_PTR(-ENODEV);
> +
> + process = domain->ops->process_alloc(task);
> + if (IS_ERR(process))
> + return process;
> + if (!process)
> + return ERR_PTR(-ENOMEM);
> +
> + process->pid= get_task_pid(task, PIDTYPE_PID);
> + process->release= domain->ops->process_free;
> + INIT_LIST_HEAD(>domains);
> + kref_init(>kref);
> +
nit, I think you should place this check right after the pid assignment.
> + if (!process->pid) {
> + err = -EINVAL;
> + goto err_free_process;
> + }
> +
> + idr_preload(GFP_KERNEL);
> + spin_lock(_process_lock);
> + pasid = idr_alloc_cyclic(_process_idr, process, domain->min_pasid,
> + domain->max_pasid + 1, GFP_ATOMIC);
> + process->pasid = pasid;
> + spin_unlock(_process_lock);
> + idr_preload_end();
> +
nit, You can maybe return here if pasid is not negative.
> + if (pasid < 0) {
> + err = pasid;
> + goto err_put_pid;
> + }
> +
> + return process;
> +
> +err_put_pid:
> + put_pid(process->pid);
> +
> +err_free_process:
> + domain->ops->process_free(process);
> +
> + return ERR_PTR(err);
> +}
> +
> +static void iommu_process_release(struct kref *kref)
> +{
> + struct iommu_process *process;
> + void (*release)(struct iommu_process *);
> +
> + assert_spin_locked(_process_lock);
> +
> + process = container_of(kref, struct iommu_process, kref);
if we are concerned about things going wrong (assert above), we should
also add some pointer check here (WARN) for process and release pointers as
well.
> + release = process->release;
> +
> + WARN_ON(!list_empty(>domains));
> +
> + idr_remove(_process_idr, process->pasid);
> + put_pid(process->pid);
> + release(process);
> +}
> +
> +/*
> + * Returns non-zero if a reference to the process was successfully taken.
> + * Returns zero if the process is being freed and should not be used.
> + */
> +static int iommu_process_get_locked(struct iommu_process *process)
> +{
> + assert_spin_locked(_process_lock);
> +
> + if (process)
> + return kref_get_unless_zero(>kref);
> +
> + return 0;
> +}
> +
> +static void iommu_process_put_locked(struct iommu_process *process)
> +{
> + assert_spin_locked(_process_lock);
> +
> + kref_put(>kref, iommu_process_release);
> +}
> +
> +static int iommu_process_attach(struct iommu_domain *domain, struct device
> *dev,
> + struct iommu_process *process)
> +{
> + int err;
> + int pasid = process->pasid;
> + struct iommu_context *context;
> +
> + if (WARN_ON(!domain->ops->process_attach ||
> !domain->ops->process_detach))
> + return -ENODEV;
> +
> + if (pasid > domain->max_pasid || pasid < domain->min_pasid)
> + return -ENOSPC;
> +
> + context = kzalloc(sizeof(*context), GFP_KERNEL);
> + if (!context)
> + return -ENOMEM;
> +
devm_kzalloc maybe?
> + context->process= process;
> + context->domain = domain;
> + refcount_set(>ref, 1);
> +
> + spin_lock(_process_lock);
> + err = domain->ops->process_attach(domain, dev, process, true);
> + if (err) {
> + kfree(context);
> + spin_unlock(_process_lock);
> + return err;
> + }
> +
> + list_add(>process_head, >domains);
> + list_add(>domain_head, >processes);
> + spin_unlock(_process_lock);
> +
> + return 0;
> +}
--
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: [RFC,20/30] iommu/arm-smmu-v3: Enable PCI PASID in masters
Hi Jean-Philippe,
> On 2/27/2017 2:54 PM, Jean-Philippe Brucker wrote:
> Enable PASID for PCI devices that support it.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.bruc...@arm.com>
> ---
> drivers/iommu/arm-smmu-v3.c | 66
> ++---
> 1 file changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 499dc1cd07eb..37fd061405e9 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -730,6 +730,8 @@ struct arm_smmu_master_data {
>
> struct arm_smmu_stream *streams;
> struct rb_root contexts;
> +
> + u32 avail_contexts;
> };
>
According to the PASID ECN here
(https://pcisig.com/sites/default/files/specification_documents/ECN-PASID-ATS-2011-03-31.pdf),
PASID should be enabled only if all switches between the root port and
a device support TLP prefix.
I'm only seeing a call to pci_enable_pasid() in this patch but I don't
see anybody checking for TLP prefix support on the hierarchy.
This could potentially be an addition to the PCI core code.
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: [RFC,20/30] iommu/arm-smmu-v3: Enable PCI PASID in masters
Hi Jean-Philippe,
On 2/27/2017 2:54 PM, Jean-Philippe Brucker wrote:
> Enable PASID for PCI devices that support it.
>
> Signed-off-by: Jean-Philippe Brucker <jean-philippe.bruc...@arm.com>
> ---
> drivers/iommu/arm-smmu-v3.c | 66
> ++---
> 1 file changed, 63 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/iommu/arm-smmu-v3.c b/drivers/iommu/arm-smmu-v3.c
> index 499dc1cd07eb..37fd061405e9 100644
> --- a/drivers/iommu/arm-smmu-v3.c
> +++ b/drivers/iommu/arm-smmu-v3.c
> @@ -730,6 +730,8 @@ struct arm_smmu_master_data {
>
> struct arm_smmu_stream *streams;
> struct rb_root contexts;
> +
> + u32 avail_contexts;
> };
>
I know that you are doing some code restructuring. As I was looking at the
amdkfd
driver, I realized that there is quite a bit of PASID, ATS and PRI work here
that
can be plumbed into the framework you are building.
https://github.com/torvalds/linux/tree/master/drivers/gpu/drm/amd/amdkfd
I wanted to share this with if you were aware of this or not. Functions of
interest
are:
amd_iommu_init_device
amd_iommu_free_device
amd_iommu_bind_pasid
amd_iommu_set_invalid_ppr_cb
amd_iommu_unbind_pasid
amd_iommu_device_info
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: [RFC] PCI ACS Flags Clarification
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 <ok...@codeaurora.org> 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
[RFC] PCI ACS Flags Clarification
I'm looking for a guideline on how Access Control Services (ACS) need to be implemented in HW for cases where peer to peer is and isn't supported. I see conflicting information in the code. iommu.c calls pci_acs_enabled() from pci_device_group() to determine if ACS path is enabled for the following flags. This paragraph lays out the requirements. /* * To consider a PCI device isolated, we require ACS to support Source * Validation, Request Redirection, Completer Redirection, and Upstream * Forwarding. This effectively means that devices cannot spoof their * requester ID, requests and completions cannot be redirected, and all * transactions are forwarded upstream, even as it passes through a * bridge where the target device is downstream. */ #define REQ_ACS_FLAGS (PCI_ACS_SV | PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF) I also see the following comment in pci_acs_flags_enabled() which gets called from pci_acs_enabled() /* * Except for egress control, capabilities are either required * or only required if controllable. Features missing from the * capability field can therefore be assumed as hard-wired enabled. */ This comment in pci_acs_flags_enabled() is confusing. A capability that reads 0 usually means it is not enabled. This code is assuming enabled. Are we trying to say that we'll use the capability bits as a mask while verifying the requested ACS flags? For systems that don't support P2P, the code will check PCI_ACS_SV only. So, if I summarize this correctly; 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? -- 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: Partial BAR Address Allocation
On 3/6/2017 6:04 AM, Joerg Roedel wrote: > On Wed, Feb 22, 2017 at 05:39:44PM -0600, Bjorn Helgaas wrote: >> [+cc Joerg, iommu list] >> >> On Wed, Feb 22, 2017 at 03:44:53PM -0500, Sinan Kaya wrote: >>> On 2/22/2017 1:44 PM, Bjorn Helgaas wrote: >>>> There is no way for a driver to say "I only need this memory BAR and >>>> not the other ones." The reason is because the PCI_COMMAND_MEMORY bit >>>> enables *all* the memory BARs; there's no way to enable memory BARs >>>> selectively. If we enable memory BARs and one of them is unassigned, >>>> that unassigned BAR is enabled, and the device will respond at >>>> whatever address the register happens to contain, and that may cause >>>> conflicts. > > Hmm, maybe I am missing something, but isn't this only a problem if the > 'unassigned' BAR as an address configured that also falls into the > Bridge-Window of the parent bridge? Otherwise no requests should be > routed to the BAR anyway, right? Correct, in order for this to happen you need to have multiple devices under a bridge. One device sends a read request towards the system address that happens to overlap with the BAR address of the unassigned BAR. The device with unassigned resource will start responding. This is one of those P2P use cases. > >>> The problem is that according to PCI specification BAR addresses and >>> DMA addresses cannot overlap. >>> >>> From PCI-to-PCI Bridge Arch. spec.: "A bridge forwards PCI memory >>> transactions from its primary interface to its secondary interface >>> (downstream) if a memory address is in the range defined by the >>> Memory Base and Memory Limit registers (when the base is less than >>> or equal to the limit) as illustrated in Figure 4-3. Conversely, a >>> memory transaction on the secondary interface that is within this >>> address range will not be forwarded upstream to the primary >>> interface." >>> >>> To be specific, if your DMA address happens to be in >>> [0x8000-0x] and root port's aperture includes this >>> range; the DMA will never make to the system memory. > > If there is no translation by an IOMMU this shouldn't be a problem, as > long as the bridge windows don't overlap with system ram. With > translation the IOMMU driver has to take care of that, which they > usually do. Correct, IOMMU drivers that I have reviewed all carve out the bridge windows out of the IOMMU driver allocatable address range in iova_reserve_pci_windows() function. > >> Hmmm. I guess SWIOTLB assumes there's no address translation in the >> DMA direction, right? If there's no address translation in the PIO >> direction, PCI bus BAR addresses are identical to the CPU-side >> addresses. In that case, there's no conflict because we already have >> to assign BARs so they never look like a system memory address. > > Yes, SWIOTLB assumes that IOVA == PA. > >> But if there *is* address translation in the PIO direction, we can >> have conflicts because the bridge can translate CPU-side PIO accesses >> to arbitrary PCI bus addresses. > > I am not aware of any hardware that does translation on the PIO space. > The IOMMUs I know of don't care about PIO at all. IOMMUs are used in the in inbound path mostly. Most of the HW has some sort of reserved address in the 4 GB whether with or without translation to support existing 32 bit only cards. We are talking about a problem in the outbound/PIO path. > > > > Joerg > > -- 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: [RFC PATCH 04/30] iommu/arm-smmu-v3: Add support for PCI ATS
On 2/27/2017 2:54 PM, Jean-Philippe Brucker wrote: > + ats_enabled = !arm_smmu_enable_ats(master); > + You should make ats_supported field in IORT table part of the decision process for when to enable ATS. -- 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: [RFC PATCH 04/30] iommu/arm-smmu-v3: Add support for PCI ATS
On 2/27/2017 2:54 PM, Jean-Philippe Brucker wrote: > /* Initialise command lazily */ > + if (!cmd.opcode) > + arm_smmu_atc_invalidate_to_cmd(smmu, iova, size, ); > + > + spin_lock(_group->devices_lock); > + > + list_for_each_entry(master, _group->devices, group_head) > + arm_smmu_atc_invalidate_master(master, ); > + > + /* > + * TODO: ensure we do a sync whenever we have sent > ats_queue_depth > + * invalidations to the same device. > + */ > + arm_smmu_cmdq_issue_cmd(smmu, _cmd); > + It is possible to observe ATS invalidation timeout up to 90 seconds according to PCIe spec. How does the current code deal with this? -- 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: Partial BAR Address Allocation
Hi Robin, On 2/23/2017 6:40 AM, Robin Murphy wrote: > On 22/02/17 23:39, Bjorn Helgaas wrote: >> [+cc Joerg, iommu list] >> >> On Wed, Feb 22, 2017 at 03:44:53PM -0500, Sinan Kaya wrote: >>> On 2/22/2017 1:44 PM, Bjorn Helgaas wrote: >>>> There is no way for a driver to say "I only need this memory BAR and >>>> not the other ones." The reason is because the PCI_COMMAND_MEMORY bit >>>> enables *all* the memory BARs; there's no way to enable memory BARs >>>> selectively. If we enable memory BARs and one of them is unassigned, >>>> that unassigned BAR is enabled, and the device will respond at >>>> whatever address the register happens to contain, and that may cause >>>> conflicts. >>>> >>>> I'm not sure this answers your question. Do you want to get rid of >>>> 32-bit BAR addresses because your host bridge doesn't have a window to >>>> 32-bit PCI addresses? It's typical for a bridge to support a window >>>> to the 32-bit PCI space as well as one to the 64-bit PCI space. Often >>>> it performs address translation for the 32-bit window so it doesn't >>>> have to be in the 32-bit area on the CPU side, e.g., you could have >>>> something like this where we have three host bridges and the 2-4GB >>>> space on each PCI root bus is addressable: >>>> >>>> pci_bus :00: root bus resource [mem 0x108000-0x10] (bus >>>> address [0x8000-0x]) >>>> pci_bus 0001:00: root bus resource [mem 0x118000-0x11] (bus >>>> address [0x8000-0x]) >>>> pci_bus 0002:00: root bus resource [mem 0x128000-0x12] (bus >>>> address [0x8000-0x]) >>> >>> The problem is that according to PCI specification BAR addresses and >>> DMA addresses cannot overlap. >>> >>> From PCI-to-PCI Bridge Arch. spec.: "A bridge forwards PCI memory >>> transactions from its primary interface to its secondary interface >>> (downstream) if a memory address is in the range defined by the >>> Memory Base and Memory Limit registers (when the base is less than >>> or equal to the limit) as illustrated in Figure 4-3. Conversely, a >>> memory transaction on the secondary interface that is within this >>> address range will not be forwarded upstream to the primary >>> interface." >>> >>> To be specific, if your DMA address happens to be in >>> [0x8000-0x] and root port's aperture includes this >>> range; the DMA will never make to the system memory. >>> >>> Lorenzo and Robin took some steps to carve out PCI addresses out of >>> DMA addresses in IOMMU drivers by using iova_reserve_pci_windows() >>> function. >>> >>> However, I see that we are still exposed when the operating system >>> doesn't have any IOMMU driver and is using the SWIOTLB for instance. >> >> Hmmm. I guess SWIOTLB assumes there's no address translation in the >> DMA direction, right? > > Not entirely - it does rely on arch-provided dma_to_phys() and > phys_to_dma() helpers which are free to accommodate such translations in > a device-specific manner. On arm64 we use these to account for > dev->dma_pfn_offset describing a straightforward linear offset, but > unless one constant offset would apply to all possible outbound windows > I'm not sure that's much help here. yeah, that won't help. This is a PCI only problem. Arch layer solution will move the entire DMA ranges for all peripherals in the SOC to a specific offset. This would be most useful if the entire DDR would start at some non-zero offset. Even then, PCI usually has several ranges. One range like this to have some space below 4GB and another untranslated range for true 64bit cards. >>>> pci_bus 0002:00: root bus resource [mem 0x128000-0x12] (bus >>>> address [0x8000-0x]) We have to emulate some range in the first 4GB to make PCI cards happy. > >> If there's no address translation in the PIO >> direction, PCI bus BAR addresses are identical to the CPU-side >> addresses. In that case, there's no conflict because we already have >> to assign BARs so they never look like a system memory address. >> >> But if there *is* address translation in the PIO direction, we can >> have conflicts because the bridge can translate CPU-side PIO accesses >> to arbitrary PCI bus addresses. >> >>> The FW solution I'm looking at requires carving out some part of the >>> DDR f
Re: [PATCH 2/2] iommu: add warning when sharing groups
Hi Alex, On 2/15/2017 4:43 PM, Sinan Kaya wrote: > On 2/15/2017 2:36 PM, Alex Williamson wrote: >> On Tue, 14 Feb 2017 22:53:35 -0500 >> ok...@codeaurora.org wrote: >> >>> On 2017-02-14 18:51, Alex Williamson wrote: >>>> On Tue, 14 Feb 2017 16:25:22 -0500 >>>> Sinan Kaya <ok...@codeaurora.org> wrote: >>>> >>>>> The ACS requirement has been obscured in the current code and is only >>>>> known by certain individuals who happen to read the code. Print out a >>>>> warning with ACS path failure when ACS requirement is not met. >>>>> >>>>> Signed-off-by: Sinan Kaya <ok...@codeaurora.org> >>>>> --- >>>>> drivers/iommu/iommu.c | 3 +++ >>>>> 1 file changed, 3 insertions(+) >>>>> >>>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>>>> index dbe7f65..049ee0a 100644 >>>>> --- a/drivers/iommu/iommu.c >>>>> +++ b/drivers/iommu/iommu.c >>>>> @@ -811,6 +811,9 @@ struct iommu_group *pci_device_group(struct device >>>>> *dev) >>>>> if (IS_ERR(group)) >>>>> return NULL; >>>>> >>>>> + if (pci_is_root_bus(bus)) >>>>> + dev_warn_once(>dev, "using shared group due to ACS path >>>>> failure\n"); >>>>> + >>>>> return group; >>>>> } >>>>> >>>> >>>> The premise here is flawed. An IOMMU group based at the root bus >>>> doesn't necessarily imply a lack of ACS. There are devices on root >>>> buses, integrated endpoints and root ports. Naturally an IOMMU group >>>> for these devices needs to be based at the root bus. Additionally, >>>> there can be IOMMU groups developed around a lack of ACS that don't >>>> intersect with the root bus. Since this is a warn_once, the false >>>> positives for root bus devices are going to be enumerated first. On an >>>> Intel system there's typically a device as 00.0 that will always be >>>> pointlessly listed first. Also, it's not clear that grouping devices >>>> together is always wrong, as Robin pointed out in the EHCI/OHCI >>>> example. Lack of ACS on downtream ports is likely to cause problems, >>>> especially if that downstream port exposes a slot. Maybe that would be >>>> a good place to start. Also, what is someone supposed to do when they >>>> see this error? If we can hope they'll look for the error in the code >>>> (unlikely) a big comment with useful external links might be >>>> necessary. Based on how easily vendors ignore kernel warnings, I'm >>>> dubious there's any value to this path though. Thanks, >>> >>> Maybe, a better solution would be to add some sentences into vfio.txt >>> documentation. >>> >>> I'm ready to drop this patch. I just don't want ACS requirement to be >>> hidden between the source code. >>> I posted V2 to linux-pci maillist but forgot to CC the iommu group. [PATCH V2] PCI: add QCOM root port quirks for ACS I dropped the second patch (this one I'm replying to) as discussed. I did minor cleanups in the first commit including 1- commit message change 2- replace dev_info_once with dev_info https://patchwork.codeaurora.org/patch/177033/ 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 2/2] iommu: add warning when sharing groups
On 2/15/2017 2:36 PM, Alex Williamson wrote: > On Tue, 14 Feb 2017 22:53:35 -0500 > ok...@codeaurora.org wrote: > >> On 2017-02-14 18:51, Alex Williamson wrote: >>> On Tue, 14 Feb 2017 16:25:22 -0500 >>> Sinan Kaya <ok...@codeaurora.org> wrote: >>> >>>> The ACS requirement has been obscured in the current code and is only >>>> known by certain individuals who happen to read the code. Print out a >>>> warning with ACS path failure when ACS requirement is not met. >>>> >>>> Signed-off-by: Sinan Kaya <ok...@codeaurora.org> >>>> --- >>>> drivers/iommu/iommu.c | 3 +++ >>>> 1 file changed, 3 insertions(+) >>>> >>>> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c >>>> index dbe7f65..049ee0a 100644 >>>> --- a/drivers/iommu/iommu.c >>>> +++ b/drivers/iommu/iommu.c >>>> @@ -811,6 +811,9 @@ struct iommu_group *pci_device_group(struct device >>>> *dev) >>>>if (IS_ERR(group)) >>>>return NULL; >>>> >>>> + if (pci_is_root_bus(bus)) >>>> + dev_warn_once(>dev, "using shared group due to ACS path >>>> failure\n"); >>>> + >>>>return group; >>>> } >>>> >>> >>> The premise here is flawed. An IOMMU group based at the root bus >>> doesn't necessarily imply a lack of ACS. There are devices on root >>> buses, integrated endpoints and root ports. Naturally an IOMMU group >>> for these devices needs to be based at the root bus. Additionally, >>> there can be IOMMU groups developed around a lack of ACS that don't >>> intersect with the root bus. Since this is a warn_once, the false >>> positives for root bus devices are going to be enumerated first. On an >>> Intel system there's typically a device as 00.0 that will always be >>> pointlessly listed first. Also, it's not clear that grouping devices >>> together is always wrong, as Robin pointed out in the EHCI/OHCI >>> example. Lack of ACS on downtream ports is likely to cause problems, >>> especially if that downstream port exposes a slot. Maybe that would be >>> a good place to start. Also, what is someone supposed to do when they >>> see this error? If we can hope they'll look for the error in the code >>> (unlikely) a big comment with useful external links might be >>> necessary. Based on how easily vendors ignore kernel warnings, I'm >>> dubious there's any value to this path though. Thanks, >> >> Maybe, a better solution would be to add some sentences into vfio.txt >> documentation. >> >> I'm ready to drop this patch. I just don't want ACS requirement to be >> hidden between the source code. >> >> Would you be willing to do that? >> >> I know I read all pci and vfio documents in the past. I could have >> captured this requirement if it was there. > > We already have this: > > Documentation/vfio.txt: > ... > This isolation is not always at the granularity of a single device > though. Even when an IOMMU is capable of this, properties of devices, > interconnects, and IOMMU topologies can each reduce this isolation. > For instance, an individual device may be part of a larger multi- > function enclosure. While the IOMMU may be able to distinguish > between devices within the enclosure, the enclosure may not require > transactions between devices to reach the IOMMU. Examples of this > could be anything from a multi-function PCI device with backdoors > between functions to a non-PCI-ACS (Access Control Services) capable > bridge allowing redirection without reaching the IOMMU. Topology > can also play a factor in terms of hiding devices. A PCIe-to-PCI > bridge masks the devices behind it, making transaction appear as if > from the bridge itself. Obviously IOMMU design plays a major factor > as well. > ... > > Additionally if you google for "iommu group", this is the first hit: > > http://vfio.blogspot.com/2014/08/iommu-groups-inside-and-out.html > > This talks extensively about ACS. A few hits below that you can find a > presentation I've given with ACS examples. What additional > documentation do you think would have helped you discover or understand > this problem earlier? > > I agree that device isolation is not a spec requirement. The specs > give us the tools that we need, but valid uses cases exist where a lack > of isolation may be preferred. If we logically deduce how we can give > a device or se
[PATCH 2/2] iommu: add warning when sharing groups
The ACS requirement has been obscured in the current code and is only known by certain individuals who happen to read the code. Print out a warning with ACS path failure when ACS requirement is not met. Signed-off-by: Sinan Kaya <ok...@codeaurora.org> --- drivers/iommu/iommu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c index dbe7f65..049ee0a 100644 --- a/drivers/iommu/iommu.c +++ b/drivers/iommu/iommu.c @@ -811,6 +811,9 @@ struct iommu_group *pci_device_group(struct device *dev) if (IS_ERR(group)) return NULL; + if (pci_is_root_bus(bus)) + dev_warn_once(>dev, "using shared group due to ACS path failure\n"); + return group; } -- 1.9.1 ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
[PATCH 1/2] PCI: add QCOM root port quirks for ACS
These QCOM root ports do provide ACS-like features to disable peer
transactions and validate bus numbers in requests, but do not provide an
actual PCIe ACS capability.
Hardware supports source validation but it will report the issue as
Completer Abort instead of ACS Violation.
Hardware doesn't support peer-to-peer and each root port is a root complex
with unique segment numbers.
It is not possible for one root port to pass traffic to the other root
port. All PCIe transactions are terminated inside the root port.
Signed-off-by: Sinan Kaya <ok...@codeaurora.org>
---
drivers/pci/quirks.c | 24
1 file changed, 24 insertions(+)
diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
index 1800bef..932949a 100644
--- a/drivers/pci/quirks.c
+++ b/drivers/pci/quirks.c
@@ -4136,6 +4136,27 @@ static int pci_quirk_intel_pch_acs(struct pci_dev *dev,
u16 acs_flags)
}
/*
+ * These QCOM root ports do provide ACS-like features to disable peer
+ * transactions and validate bus numbers in requests, but do not provide an
+ * actual PCIe ACS capability.
+ * Hardware supports source validation but it will report the issue as
+ * Completer Abort instead of ACS Violation.
+ * Hardware doesn't support peer-to-peer and each root port is a root complex
+ * with unique segment numbers.
+ * It is not possible for one root port to pass traffic to the other root
+ * port. All PCIe transactions are terminated inside the root port.
+ */
+static int pci_quirk_qcom_rp_acs(struct pci_dev *dev, u16 acs_flags)
+{
+ u16 flags = (PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_UF | PCI_ACS_SV);
+ int ret = acs_flags & ~flags ? 0 : 1;
+
+ dev_info_once(>dev, "Using QCOM ACS Quirk (%d)\n", ret);
+
+ return ret;
+}
+
+/*
* Sunrise Point PCH root ports implement ACS, but unfortunately as shown in
* the datasheet (Intel 100 Series Chipset Family PCH Datasheet, Vol. 2,
* 12.1.46, 12.1.47)[1] this chipset uses dwords for the ACS capability and
@@ -4271,6 +4292,9 @@ static int pci_quirk_mf_endpoint_acs(struct pci_dev *dev,
u16 acs_flags)
/* I219 */
{ PCI_VENDOR_ID_INTEL, 0x15b7, pci_quirk_mf_endpoint_acs },
{ PCI_VENDOR_ID_INTEL, 0x15b8, pci_quirk_mf_endpoint_acs },
+ /* QCOM QDF2xxx root ports */
+ { 0x17CB, 0x400, pci_quirk_qcom_rp_acs },
+ { 0x17CB, 0x401, pci_quirk_qcom_rp_acs },
/* Intel PCH root ports */
{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_pch_acs },
{ PCI_VENDOR_ID_INTEL, PCI_ANY_ID, pci_quirk_intel_spt_pch_acs },
--
1.9.1
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: RFC on No ACS Support and SMMUv3 Support
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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: RFC on No ACS Support and SMMUv3 Support
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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: RFC on No ACS Support and SMMUv3 Support
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 <ok...@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
RFC on No ACS Support and SMMUv3 Support
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. ___ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu
Re: [PATCH V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 2/1/2017 1:52 PM, Lorenzo Pieralisi wrote: >> Sure, send it to both Nate and me. > I debugged the issue and Nate's fix is correct, the fact that you > can't it hit it with mainline is just a matter of timing because it has > to do with the CTX pointer value (we OR it with the existing value), so > it may work or not depending on how the cdptr memory allocation > pattern turns out to be (which explains why Nate and I can hit it with > simple PCI device remove/add execution too). > > So it is neither an ACPI nor an IOMMU probe deferral issue per-se, > fix is already queued, so it is all good. > > What about USB stalls ? > > Thanks ! > Lorenzo > I'll get a hold of Nate and debug this. We were waiting for you to send us debug code. -- 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 V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 1/30/2017 11:51 AM, Lorenzo Pieralisi wrote: > On Mon, Jan 30, 2017 at 10:46:39AM -0500, Sinan Kaya wrote: >> On 1/30/2017 9:54 AM, Nate Watterson wrote: >>> On 2017-01-30 09:38, Will Deacon wrote: >>>> On Mon, Jan 30, 2017 at 09:33:50AM -0500, Sinan Kaya wrote: >>>>> On 1/30/2017 9:23 AM, Nate Watterson wrote: >>>>>> On 2017-01-30 08:59, Sinan Kaya wrote: >>>>>>> On 1/30/2017 7:22 AM, Robin Murphy wrote: >>>>>>>> On 29/01/17 17:53, Sinan Kaya wrote: >>>>>>>>> On 1/24/2017 7:37 AM, Lorenzo Pieralisi wrote: >>>>>>>>>> [+hanjun, tomasz, sinan] >>>>>>>>>> >>>>>>>>>> It is quite a key patchset, I would be glad if they can test on their >>>>>>>>>> respective platforms with IORT. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Tested on top of 4.10-rc5. >>>>>>>>> >>>>>>>>> 1.Platform Hidma device passed dmatest >>>>>>>>> 2.Seeing some USB stalls on a platform USB device. >>>>>>>>> 3.PCIe NVME drive probed and worked fine with MSI interrupts >>>>>>>>> after boot. >>>>>>>>> 4. NVMe driver didn't probe following a hotplug insertion and >>>>>>>>> received an >>>>>>>>> SMMU error event during the insertion. >>>>>>>> >>>>>>>> What was the SMMU error - a translation/permission fault (implying the >>>>>>>> wrong DMA ops) or a bad STE fault (implying we totally failed to tell >>>>>>>> the SMMU about the device at all)? >>>>>>>> >>>>>>> >>>>>>> root@ubuntu:/sys/bus/pci/slots/4# echo 0 > power >>>>>>> >>>>>>> [__204.698522]_iommu:_Removing_device_0003:01:00.0_from_group_0 >>>>>>> [ 204.708704] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down >>>>>>> [ 204.708723] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down event >>>>>>> ignored; already powering off >>>>>>> >>>>>>> root@ubuntu:/sys/bus/pci/slots/4# >>>>>>> >>>>>>> [__254.820440]_iommu:_Adding_device_0003:01:00.0_to_group_8 >>>>>>> [ 254.820599] nvme nvme0: pci function 0003:01:00.0 >>>>>>> [ 254.820621] nvme 0003:01:00.0: enabling device ( -> 0002) >>>>>>> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >>>>>>> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x010a >>>>>>> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>>>>>> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>>>>>> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>>>> >>>>>> Looks like C_BAD_CD. Can you please try with: >>>>>> iommu/arm-smmu-v3: Clear prior settings when updating STEs >>>>> >>>>> This resolved the issue. Can we pull Nate's patch to 4.10 so that I don't >>>>> see >>>>> this issue again. >>>> >>>> I already sent the pull request to Joerg for 4.11. Do you see this problem >>>> without Sricharan's patches (i.e. vanilla mainline)? If so, we'll need to >>>> send the patch to stable after -rc1. >>> Using vanilla mainline, I see it most commonly when directly assigning >>> a device to a guest machine. I think I've also seen it after removing then >>> re-adding a PCI device. Basically anytime an STE's CTX pointer is changed >>> from a non-NULL value and STE[CFG] indicates translation will be performed. >>> >> >> I was not able to reproduce the issue with Vanilla kernel. I only >> tested hotplug. > > I would like to get the complete code path leading to this issue, it is > not clear to me why the probe deferral code triggers it and why we are > not able to trigger it with vanilla mainline, we must understand that first > before applying any fix to this series. > > I do not have a platform to reproduce this issue I will send you a patch > to trace what's going on here please help us debug it. Sure, send it to both Nate and me. > > Thanks, > Lorenzo > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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 V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 1/30/2017 9:54 AM, Nate Watterson wrote: > On 2017-01-30 09:38, Will Deacon wrote: >> On Mon, Jan 30, 2017 at 09:33:50AM -0500, Sinan Kaya wrote: >>> On 1/30/2017 9:23 AM, Nate Watterson wrote: >>> > On 2017-01-30 08:59, Sinan Kaya wrote: >>> >> On 1/30/2017 7:22 AM, Robin Murphy wrote: >>> >>> On 29/01/17 17:53, Sinan Kaya wrote: >>> >>>> On 1/24/2017 7:37 AM, Lorenzo Pieralisi wrote: >>> >>>>> [+hanjun, tomasz, sinan] >>> >>>>> >>> >>>>> It is quite a key patchset, I would be glad if they can test on their >>> >>>>> respective platforms with IORT. >>> >>>>> >>> >>>> >>> >>>> Tested on top of 4.10-rc5. >>> >>>> >>> >>>> 1.Platform Hidma device passed dmatest >>> >>>> 2.Seeing some USB stalls on a platform USB device. >>> >>>> 3.PCIe NVME drive probed and worked fine with MSI interrupts after >>> >>>> boot. >>> >>>> 4. NVMe driver didn't probe following a hotplug insertion and >>> >>>> received an >>> >>>> SMMU error event during the insertion. >>> >>> >>> >>> What was the SMMU error - a translation/permission fault (implying the >>> >>> wrong DMA ops) or a bad STE fault (implying we totally failed to tell >>> >>> the SMMU about the device at all)? >>> >>> >>> >> >>> >> root@ubuntu:/sys/bus/pci/slots/4# echo 0 > power >>> >> >>> >> [__204.698522]_iommu:_Removing_device_0003:01:00.0_from_group_0 >>> >> [ 204.708704] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down >>> >> [ 204.708723] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down event >>> >> ignored; already powering off >>> >> >>> >> root@ubuntu:/sys/bus/pci/slots/4# >>> >> >>> >> [__254.820440]_iommu:_Adding_device_0003:01:00.0_to_group_8 >>> >> [ 254.820599] nvme nvme0: pci function 0003:01:00.0 >>> >> [ 254.820621] nvme 0003:01:00.0: enabling device ( -> 0002) >>> >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >>> >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x010a >>> >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>> >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>> >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >>> >>> > Looks like C_BAD_CD. Can you please try with: >>> > iommu/arm-smmu-v3: Clear prior settings when updating STEs >>> >>> This resolved the issue. Can we pull Nate's patch to 4.10 so that I don't >>> see >>> this issue again. >> >> I already sent the pull request to Joerg for 4.11. Do you see this problem >> without Sricharan's patches (i.e. vanilla mainline)? If so, we'll need to >> send the patch to stable after -rc1. > Using vanilla mainline, I see it most commonly when directly assigning > a device to a guest machine. I think I've also seen it after removing then > re-adding a PCI device. Basically anytime an STE's CTX pointer is changed > from a non-NULL value and STE[CFG] indicates translation will be performed. > I was not able to reproduce the issue with Vanilla kernel. I only tested hotplug. > Nate >> >> Will > -- 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 V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 1/30/2017 9:23 AM, Nate Watterson wrote: > On 2017-01-30 08:59, Sinan Kaya wrote: >> On 1/30/2017 7:22 AM, Robin Murphy wrote: >>> On 29/01/17 17:53, Sinan Kaya wrote: >>>> On 1/24/2017 7:37 AM, Lorenzo Pieralisi wrote: >>>>> [+hanjun, tomasz, sinan] >>>>> >>>>> It is quite a key patchset, I would be glad if they can test on their >>>>> respective platforms with IORT. >>>>> >>>> >>>> Tested on top of 4.10-rc5. >>>> >>>> 1.Platform Hidma device passed dmatest >>>> 2.Seeing some USB stalls on a platform USB device. >>>> 3.PCIe NVME drive probed and worked fine with MSI interrupts after >>>> boot. >>>> 4. NVMe driver didn't probe following a hotplug insertion and received >>>> an >>>> SMMU error event during the insertion. >>> >>> What was the SMMU error - a translation/permission fault (implying the >>> wrong DMA ops) or a bad STE fault (implying we totally failed to tell >>> the SMMU about the device at all)? >>> >> >> root@ubuntu:/sys/bus/pci/slots/4# echo 0 > power >> >> [__204.698522]_iommu:_Removing_device_0003:01:00.0_from_group_0 >> [ 204.708704] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down >> [ 204.708723] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down event >> ignored; already powering off >> >> root@ubuntu:/sys/bus/pci/slots/4# >> >> [__254.820440]_iommu:_Adding_device_0003:01:00.0_to_group_8 >> [ 254.820599] nvme nvme0: pci function 0003:01:00.0 >> [ 254.820621] nvme 0003:01:00.0: enabling device ( -> 0002) >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x010a >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x > Looks like C_BAD_CD. Can you please try with: > iommu/arm-smmu-v3: Clear prior settings when updating STEs This resolved the issue. Can we pull Nate's patch to 4.10 so that I don't see this issue again. root@ubuntu:/sys/bus/pci/slots# cd 4 root@ubuntu:/sys/bus/pci/slots/4# ls adapter address attention cur_bus_speed latch max_bus_speed module power root@ubuntu:/sys/bus/pci/slots/4# echo 0 > power root@ubuntu:/sys/bus/pci/slots/4# echo 1 > power root@ubuntu:/sys/bus/pci/slots/4# dmesg |tail -n 10 [ 44.136028] pci 0003:01:00.0: BAR 0: assigned [mem 0xc010011-0xc0100113fff 64bit] [ 44.136044] pcieport 0003:00:00.0: PCI bridge to [bus 01] [ 44.136046] pcieport 0003:00:00.0: bridge window [io 0x1-0x10fff] [ 44.136048] pcieport 0003:00:00.0: bridge window [mem 0xc010010-0xc01002f] [ 44.136050] pcieport 0003:00:00.0: bridge window [mem 0xc04-0xc04001f 64bit pref] [ 44.136059] pcieport 0003:00:00.0: Max Payload Size set to 256/ 512 (was 256), Max Read Rq 512 [ 44.136073] pci 0003:01:00.0: Max Payload Size set to 256/ 256 (was 128), Max Read Rq 512 [ 44.136124] iommu: Adding device 0003:01:00.0 to group 8 [ 44.136275] nvme nvme0: pci function 0003:01:00.0 [ 44.136292] nvme 0003:01:00.0: enabling device ( -> 0002) root@ubuntu:/sys/bus/pci/slots/4# ls /dev/nvme* /dev/nvme0 /dev/nvme0n1 root@ubuntu:/sys/bus/pci/slots/4# I'll look at the USB stalls next. -- 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 V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 1/30/2017 7:22 AM, Robin Murphy wrote: > On 29/01/17 17:53, Sinan Kaya wrote: >> On 1/24/2017 7:37 AM, Lorenzo Pieralisi wrote: >>> [+hanjun, tomasz, sinan] >>> >>> It is quite a key patchset, I would be glad if they can test on their >>> respective platforms with IORT. >>> >> >> Tested on top of 4.10-rc5. >> >> 1. Platform Hidma device passed dmatest >> 2. Seeing some USB stalls on a platform USB device. >> 3. PCIe NVME drive probed and worked fine with MSI interrupts after boot. >> 4. NVMe driver didn't probe following a hotplug insertion and received an >> SMMU error event during the insertion. > > What was the SMMU error - a translation/permission fault (implying the > wrong DMA ops) or a bad STE fault (implying we totally failed to tell > the SMMU about the device at all)? > root@ubuntu:/sys/bus/pci/slots/4# echo 0 > power [__204.698522]_iommu:_Removing_device_0003:01:00.0_from_group_0 [ 204.708704] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down [ 204.708723] pciehp 0003:00:00.0:pcie004: Slot(4): Link Down event ignored; already powering off root@ubuntu:/sys/bus/pci/slots/4# [__254.820440]_iommu:_Adding_device_0003:01:00.0_to_group_8 [ 254.820599] nvme nvme0: pci function 0003:01:00.0 [ 254.820621] nvme 0003:01:00.0: enabling device ( -> 0002) [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x010a [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x root@ubuntu:/sys/bus/pci/slots/4# root@ubuntu:/sys/bus/pci/slots/4#ls /dev/nvme* /dev/nvme0 I should have seen /dev/nvme0n1 partition here. > Robin. > >> >> /sys/bus/pci/slots/4 # >> /sys/bus/pci/slots/4 # dmesg | grep nvme >> [ 14.041357] nvme nvme0: pci function 0003:01:00.0 >> [ 198.399521] nvme nvme0: pci function 0003:01:00.0 >> [__198.416232]_nvme_0003:01:00.0:_enabling_device_(_->_0002) >> [ 264.402216] nvme nvme0: I/O 228 QID 0 timeout, disable controller >> [ 264.402313] nvme nvme0: Identify Controller failed (-4) >> [ 264.421270] nvme nvme0: Removing after probe failure status: -5 >> /sys/bus/pci/slots/4 # >> >> >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-pci" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- 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 V7 08/11] drivers: acpi: Handle IOMMU lookup failure with deferred probing or error
On 1/24/2017 7:37 AM, Lorenzo Pieralisi wrote: > [+hanjun, tomasz, sinan] > > It is quite a key patchset, I would be glad if they can test on their > respective platforms with IORT. > Tested on top of 4.10-rc5. 1. Platform Hidma device passed dmatest 2. Seeing some USB stalls on a platform USB device. 3. PCIe NVME drive probed and worked fine with MSI interrupts after boot. 4. NVMe driver didn't probe following a hotplug insertion and received an SMMU error event during the insertion. /sys/bus/pci/slots/4 # /sys/bus/pci/slots/4 # dmesg | grep nvme [ 14.041357] nvme nvme0: pci function 0003:01:00.0 [ 198.399521] nvme nvme0: pci function 0003:01:00.0 [__198.416232]_nvme_0003:01:00.0:_enabling_device_(_->_0002) [ 264.402216] nvme nvme0: I/O 228 QID 0 timeout, disable controller [ 264.402313] nvme nvme0: Identify Controller failed (-4) [ 264.421270] nvme nvme0: Removing after probe failure status: -5 /sys/bus/pci/slots/4 # -- 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 V7 07/11] iommu: of: Handle IOMMU lookup failure with deferred probing or error
On 1/23/2017 11:18 AM, Sricharan R wrote:
> @@ -107,7 +107,7 @@ void of_dma_configure(struct device *dev, struct
> device_node *np)
> ret = of_dma_get_range(np, _addr, , );
> if (ret < 0) {
> dma_addr = offset = 0;
> - size = dev->coherent_dma_mask + 1;
> + size = max(dev->coherent_dma_mask, dev->coherent_dma_mask + 1);
> } else {
what's happening here?
--
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
