| Message ID | 1485188293-20263-9-git-send-email-sricharan@codeaurora.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
[+hanjun, tomasz, sinan] It is quite a key patchset, I would be glad if they can test on their respective platforms with IORT. On Mon, Jan 23, 2017 at 09:48:10PM +0530, Sricharan R wrote: > This is an equivalent to the DT's handling of the iommu master's probe > with deferred probing when the corrsponding iommu is not probed yet. > The lack of a registered IOMMU can be caused by the lack of a driver for > the IOMMU, the IOMMU device probe not having been performed yet, having > been deferred, or having failed. > > The first case occurs when the firmware describes the bus master and > IOMMU topology correctly but no device driver exists for the IOMMU yet > or the device driver has not been compiled in. Return NULL, the caller > will configure the device without an IOMMU. > > The second and third cases are handled by deferring the probe of the bus > master device which will eventually get reprobed after the IOMMU. > > The last case is currently handled by deferring the probe of the bus > master device as well. A mechanism to either configure the bus master > device without an IOMMU or to fail the bus master device probe depending > on whether the IOMMU is optional or mandatory would be a good > enhancement. > > Signed-off-by: Sricharan R <sricharan@codeaurora.org> > --- > drivers/acpi/arm64/iort.c | 25 ++++++++++++++++++++++++- > drivers/acpi/scan.c | 7 +++++-- > drivers/base/dma-mapping.c | 2 +- > include/acpi/acpi_bus.h | 2 +- > include/linux/acpi.h | 7 +++++-- > 5 files changed, 36 insertions(+), 7 deletions(-) > > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c > index bf0ed09..d01bae8 100644 > --- a/drivers/acpi/arm64/iort.c > +++ b/drivers/acpi/arm64/iort.c > @@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev, > return NULL; > > ops = iommu_get_instance(iort_fwnode); > + /* > + * If the ops look-up fails, this means that either > + * the SMMU drivers have not been probed yet or that > + * the SMMU drivers are not built in the kernel; > + * Depending on whether the SMMU drivers are built-in > + * in the kernel or not, defer the IOMMU configuration > + * or just abort it. > + */ > if (!ops) > - return NULL; > + return iort_iommu_driver_enabled(node->type) ? > + ERR_PTR(-EPROBE_DEFER) : NULL; > > ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops); > } > @@ -625,12 +634,26 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) > > while (parent) { > ops = iort_iommu_xlate(dev, parent, streamid); > + if (IS_ERR_OR_NULL(ops)) > + return ops; > > parent = iort_node_get_id(node, &streamid, > IORT_IOMMU_TYPE, i++); > } > } > > + /* > + * If we have reason to believe the IOMMU driver missed the initial > + * add_device callback for dev, replay it to get things in order. > + */ > + if (!IS_ERR_OR_NULL(ops) && ops->add_device && > + dev->bus && !dev->iommu_group) { > + int err = ops->add_device(dev); > + > + if (err) > + ops = ERR_PTR(err); > + } I think there is nothing ACPI specific in this add_device() replay path, so there is room for further DT/ACPI consolidation here. Without any further ado: Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> > + > return ops; > } > > diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c > index 1926918..823b005 100644 > --- a/drivers/acpi/scan.c > +++ b/drivers/acpi/scan.c > @@ -1373,20 +1373,23 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) > * @dev: The pointer to the device > * @attr: device dma attributes > */ > -void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) > +int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) > { > const struct iommu_ops *iommu; > > iort_set_dma_mask(dev); > > iommu = iort_iommu_configure(dev); > - > + if (IS_ERR(iommu)) > + return PTR_ERR(iommu); > /* > * Assume dma valid range starts at 0 and covers the whole > * coherent_dma_mask. > */ > arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu, > attr == DEV_DMA_COHERENT); > + > + return 0; > } > EXPORT_SYMBOL_GPL(acpi_dma_configure); > > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > index 82bd45c..755a2b5 100644 > --- a/drivers/base/dma-mapping.c > +++ b/drivers/base/dma-mapping.c > @@ -368,7 +368,7 @@ int dma_configure(struct device *dev) > } 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) > - acpi_dma_configure(dev, attr); > + ret = acpi_dma_configure(dev, attr); > } > > if (bridge) > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index 4242c31..9aa762fe 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -573,7 +573,7 @@ struct acpi_pci_root { > > bool acpi_dma_supported(struct acpi_device *adev); > enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev); > -void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr); > +int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr); > void acpi_dma_deconfigure(struct device *dev); > > struct acpi_device *acpi_find_child_device(struct acpi_device *parent, > diff --git a/include/linux/acpi.h b/include/linux/acpi.h > index 5b36974..8b958b6 100644 > --- a/include/linux/acpi.h > +++ b/include/linux/acpi.h > @@ -764,8 +764,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) > return DEV_DMA_NOT_SUPPORTED; > } > > -static inline void acpi_dma_configure(struct device *dev, > - enum dev_dma_attr attr) { } > +static inline int acpi_dma_configure(struct device *dev, > + enum dev_dma_attr attr) > +{ > + return 0; > +} > > static inline void acpi_dma_deconfigure(struct device *dev) { } > > -- > QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation >
Hi Lorenzo, Sricharan, On 01/24/2017 08:37 PM, 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. ACPI patches are conflict with my acpi platform msi patches (I need them to enable devices on my ARM64 platform then test this patch set), I will fix the conflict and test it tomorrow (out of office now and can't reach the test machines) :) Thanks Hanjun
Hi Lorenzo, >[+hanjun, tomasz, sinan] > >It is quite a key patchset, I would be glad if they can test on their >respective platforms with IORT. > >On Mon, Jan 23, 2017 at 09:48:10PM +0530, Sricharan R wrote: >> This is an equivalent to the DT's handling of the iommu master's probe >> with deferred probing when the corrsponding iommu is not probed yet. >> The lack of a registered IOMMU can be caused by the lack of a driver for >> the IOMMU, the IOMMU device probe not having been performed yet, having >> been deferred, or having failed. >> >> The first case occurs when the firmware describes the bus master and >> IOMMU topology correctly but no device driver exists for the IOMMU yet >> or the device driver has not been compiled in. Return NULL, the caller >> will configure the device without an IOMMU. >> >> The second and third cases are handled by deferring the probe of the bus >> master device which will eventually get reprobed after the IOMMU. >> >> The last case is currently handled by deferring the probe of the bus >> master device as well. A mechanism to either configure the bus master >> device without an IOMMU or to fail the bus master device probe depending >> on whether the IOMMU is optional or mandatory would be a good >> enhancement. >> >> Signed-off-by: Sricharan R <sricharan@codeaurora.org> >> --- >> drivers/acpi/arm64/iort.c | 25 ++++++++++++++++++++++++- >> drivers/acpi/scan.c | 7 +++++-- >> drivers/base/dma-mapping.c | 2 +- >> include/acpi/acpi_bus.h | 2 +- >> include/linux/acpi.h | 7 +++++-- >> 5 files changed, 36 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c >> index bf0ed09..d01bae8 100644 >> --- a/drivers/acpi/arm64/iort.c >> +++ b/drivers/acpi/arm64/iort.c >> @@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev, >> return NULL; >> >> ops = iommu_get_instance(iort_fwnode); >> + /* >> + * If the ops look-up fails, this means that either >> + * the SMMU drivers have not been probed yet or that >> + * the SMMU drivers are not built in the kernel; >> + * Depending on whether the SMMU drivers are built-in >> + * in the kernel or not, defer the IOMMU configuration >> + * or just abort it. >> + */ >> if (!ops) >> - return NULL; >> + return iort_iommu_driver_enabled(node->type) ? >> + ERR_PTR(-EPROBE_DEFER) : NULL; >> >> ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops); >> } >> @@ -625,12 +634,26 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) >> >> while (parent) { >> ops = iort_iommu_xlate(dev, parent, streamid); >> + if (IS_ERR_OR_NULL(ops)) >> + return ops; >> >> parent = iort_node_get_id(node, &streamid, >> IORT_IOMMU_TYPE, i++); >> } >> } >> >> + /* >> + * If we have reason to believe the IOMMU driver missed the initial >> + * add_device callback for dev, replay it to get things in order. >> + */ >> + if (!IS_ERR_OR_NULL(ops) && ops->add_device && >> + dev->bus && !dev->iommu_group) { >> + int err = ops->add_device(dev); >> + >> + if (err) >> + ops = ERR_PTR(err); >> + } > >I think there is nothing ACPI specific in this add_device() replay >path, so there is room for further DT/ACPI consolidation here. > ok, the only way is keep this in a function and call it for both DT and ACPI cases. >Without any further ado: > >Acked-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com> Thanks for that. Regards, Sricharan
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_(0000_->_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 #
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)? 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_(0000_->_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 # > > >
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 (0000 -> 0002) [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 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_(0000_->_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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
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 (0000 -> 0002) > [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: > [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a > [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 Looks like C_BAD_CD. Can you please try with: iommu/arm-smmu-v3: Clear prior settings when updating STEs > 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_(0000_->_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 majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html >>
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 (0000 -> 0002) >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > 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 0xc0100110000-0xc0100113fff 64bit] [ 44.136044] pcieport 0003:00:00.0: PCI bridge to [bus 01] [ 44.136046] pcieport 0003:00:00.0: bridge window [io 0x10000-0x10fff] [ 44.136048] pcieport 0003:00:00.0: bridge window [mem 0xc0100100000-0xc01002fffff] [ 44.136050] pcieport 0003:00:00.0: bridge window [mem 0xc0400000000-0xc04001fffff 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 (0000 -> 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.
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 (0000 -> 0002) > >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: > >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a > >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > > > 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. Will
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 (0000 -> 0002) >> >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >> >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a >> >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >> >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >> >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >> >> > 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. Nate > > Will
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 (0000 -> 0002) >>> >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >>> >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a >>> >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>> >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>> >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>> >>> > 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 >
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 (0000 -> 0002) > >>> >> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: > >>> >> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a > >>> >> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>> >> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>> >> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>> > >>> > 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. Thanks, Lorenzo
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 (0000 -> 0002) >>>>>>> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: >>>>>>> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a >>>>>>> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>>>>>> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>>>>>> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 >>>>> >>>>>> 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 majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html >
On Mon, Jan 30, 2017 at 03:03:06PM -0500, Sinan Kaya wrote: > 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 (0000 -> 0002) > >>>>>>> [ 261.948558] arm-smmu-v3 arm-smmu-v3.0.auto: event 0x0a received: > >>>>>>> [ 261.948561] arm-smmu-v3 arm-smmu-v3.0.auto: 0x000001000000000a > >>>>>>> [ 261.948563] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>>>>>> [ 261.948564] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>>>>>> [ 261.948566] arm-smmu-v3 arm-smmu-v3.0.auto: 0x0000000000000000 > >>>>> > >>>>>> 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. 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
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.
On 2017-02-01 13:52, Lorenzo Pieralisi wrote: > > 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 ? Our fault. The USB controller was getting 48-bit IOVAs, but the bus it sits on only supports 44-bits so the controller's DMA transactions ended up targeting addresses that were never mapped. It started working once I applied the iort/dma-mapping patches I sent out earlier this week that use the iort memory_address_limit field to check if a dma_mask is sane. Sorry for the false alarm. Nate > > Thanks ! > Lorenzo
On 02/03/2017 03:01 AM, Nate Watterson wrote: > On 2017-02-01 13:52, Lorenzo Pieralisi wrote: >> >> 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 ? > Our fault. The USB controller was getting 48-bit IOVAs, but the > bus it sits on only supports 44-bits so the controller's DMA > transactions ended up targeting addresses that were never mapped. > > It started working once I applied the iort/dma-mapping patches I > sent out earlier this week that use the iort memory_address_limit > field to check if a dma_mask is sane. OK, great, I tested this patch with platform USB device which was working fine on Hisilicon D03, so I didn't miss anything here. Thanks Hanjun
Hi, >On 2017-02-01 13:52, Lorenzo Pieralisi wrote: >> >> 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 ? >Our fault. The USB controller was getting 48-bit IOVAs, but the >bus it sits on only supports 44-bits so the controller's DMA >transactions ended up targeting addresses that were never mapped. > >It started working once I applied the iort/dma-mapping patches I >sent out earlier this week that use the iort memory_address_limit >field to check if a dma_mask is sane. > >Sorry for the false alarm. Ok, thanks for closing on it. I will just post V8 with all acks picked up and Robin's fix , right away. Regards, Sricharan
diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c index bf0ed09..d01bae8 100644 --- a/drivers/acpi/arm64/iort.c +++ b/drivers/acpi/arm64/iort.c @@ -550,8 +550,17 @@ static const struct iommu_ops *iort_iommu_xlate(struct device *dev, return NULL; ops = iommu_get_instance(iort_fwnode); + /* + * If the ops look-up fails, this means that either + * the SMMU drivers have not been probed yet or that + * the SMMU drivers are not built in the kernel; + * Depending on whether the SMMU drivers are built-in + * in the kernel or not, defer the IOMMU configuration + * or just abort it. + */ if (!ops) - return NULL; + return iort_iommu_driver_enabled(node->type) ? + ERR_PTR(-EPROBE_DEFER) : NULL; ret = arm_smmu_iort_xlate(dev, streamid, iort_fwnode, ops); } @@ -625,12 +634,26 @@ const struct iommu_ops *iort_iommu_configure(struct device *dev) while (parent) { ops = iort_iommu_xlate(dev, parent, streamid); + if (IS_ERR_OR_NULL(ops)) + return ops; parent = iort_node_get_id(node, &streamid, IORT_IOMMU_TYPE, i++); } } + /* + * If we have reason to believe the IOMMU driver missed the initial + * add_device callback for dev, replay it to get things in order. + */ + if (!IS_ERR_OR_NULL(ops) && ops->add_device && + dev->bus && !dev->iommu_group) { + int err = ops->add_device(dev); + + if (err) + ops = ERR_PTR(err); + } + return ops; } diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 1926918..823b005 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -1373,20 +1373,23 @@ enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) * @dev: The pointer to the device * @attr: device dma attributes */ -void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) +int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr) { const struct iommu_ops *iommu; iort_set_dma_mask(dev); iommu = iort_iommu_configure(dev); - + if (IS_ERR(iommu)) + return PTR_ERR(iommu); /* * Assume dma valid range starts at 0 and covers the whole * coherent_dma_mask. */ arch_setup_dma_ops(dev, 0, dev->coherent_dma_mask + 1, iommu, attr == DEV_DMA_COHERENT); + + return 0; } EXPORT_SYMBOL_GPL(acpi_dma_configure); diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index 82bd45c..755a2b5 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -368,7 +368,7 @@ int dma_configure(struct device *dev) } 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) - acpi_dma_configure(dev, attr); + ret = acpi_dma_configure(dev, attr); } if (bridge) diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 4242c31..9aa762fe 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -573,7 +573,7 @@ struct acpi_pci_root { bool acpi_dma_supported(struct acpi_device *adev); enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev); -void acpi_dma_configure(struct device *dev, enum dev_dma_attr attr); +int acpi_dma_configure(struct device *dev, enum dev_dma_attr attr); void acpi_dma_deconfigure(struct device *dev); struct acpi_device *acpi_find_child_device(struct acpi_device *parent, diff --git a/include/linux/acpi.h b/include/linux/acpi.h index 5b36974..8b958b6 100644 --- a/include/linux/acpi.h +++ b/include/linux/acpi.h @@ -764,8 +764,11 @@ static inline enum dev_dma_attr acpi_get_dma_attr(struct acpi_device *adev) return DEV_DMA_NOT_SUPPORTED; } -static inline void acpi_dma_configure(struct device *dev, - enum dev_dma_attr attr) { } +static inline int acpi_dma_configure(struct device *dev, + enum dev_dma_attr attr) +{ + return 0; +} static inline void acpi_dma_deconfigure(struct device *dev) { }
This is an equivalent to the DT's handling of the iommu master's probe with deferred probing when the corrsponding iommu is not probed yet. The lack of a registered IOMMU can be caused by the lack of a driver for the IOMMU, the IOMMU device probe not having been performed yet, having been deferred, or having failed. The first case occurs when the firmware describes the bus master and IOMMU topology correctly but no device driver exists for the IOMMU yet or the device driver has not been compiled in. Return NULL, the caller will configure the device without an IOMMU. The second and third cases are handled by deferring the probe of the bus master device which will eventually get reprobed after the IOMMU. The last case is currently handled by deferring the probe of the bus master device as well. A mechanism to either configure the bus master device without an IOMMU or to fail the bus master device probe depending on whether the IOMMU is optional or mandatory would be a good enhancement. Signed-off-by: Sricharan R <sricharan@codeaurora.org> --- drivers/acpi/arm64/iort.c | 25 ++++++++++++++++++++++++- drivers/acpi/scan.c | 7 +++++-- drivers/base/dma-mapping.c | 2 +- include/acpi/acpi_bus.h | 2 +- include/linux/acpi.h | 7 +++++-- 5 files changed, 36 insertions(+), 7 deletions(-)