Re: [PATCH] dma-mapping: skip USB devices when configuring DMA during probe
On Fri, Aug 04, 2017 at 03:40:42AM +0800, kbuild test robot wrote: > Hi Johan, > > [auto build test ERROR on driver-core/driver-core-testing] > [also build test ERROR on v4.13-rc3 next-20170803] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Johan-Hovold/dma-mapping-skip-USB-devices-when-configuring-DMA-during-probe/20170804-014620 > config: microblaze-mmu_defconfig (attached as .config) > compiler: microblaze-linux-gcc (GCC) 6.2.0 > reproduce: > wget > https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O > ~/bin/make.cross > chmod +x ~/bin/make.cross > # save the attached .config to linux build tree > make.cross ARCH=microblaze > > All errors (new ones prefixed by >>): > >drivers/base/dma-mapping.o: In function `dma_configure': > >> drivers/base/dma-mapping.c:350: undefined reference to `usb_bus_type' >drivers/base/dma-mapping.o: In function `dma_deconfigure': >drivers/base/dma-mapping.c:377: undefined reference to `usb_bus_type' >net/sunrpc/stats.o: In function `rpc_print_iostats': >net/sunrpc/stats.c:206: undefined reference to `_GLOBAL_OFFSET_TABLE_' >scripts/link-vmlinux.sh: line 93: 76193 Segmentation fault ${LD} > ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${objects} Thanks for the report. Alan Stern pointed this out during review, so this patch has already been superseded. Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-mapping: skip USB devices when configuring DMA during probe
Hi Johan, [auto build test ERROR on driver-core/driver-core-testing] [also build test ERROR on v4.13-rc3 next-20170803] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Johan-Hovold/dma-mapping-skip-USB-devices-when-configuring-DMA-during-probe/20170804-014620 config: sparc-defconfig (attached as .config) compiler: sparc-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sparc All errors (new ones prefixed by >>): drivers/base/dma-mapping.o: In function `dma_configure': >> dma-mapping.c:(.text+0x720): undefined reference to `usb_bus_type' dma-mapping.c:(.text+0x728): undefined reference to `usb_bus_type' drivers/base/dma-mapping.o: In function `dma_deconfigure': dma-mapping.c:(.text+0x7d0): undefined reference to `usb_bus_type' dma-mapping.c:(.text+0x7d4): undefined reference to `usb_bus_type' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] dma-mapping: skip USB devices when configuring DMA during probe
Hi Johan, [auto build test ERROR on driver-core/driver-core-testing] [also build test ERROR on v4.13-rc3 next-20170803] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Johan-Hovold/dma-mapping-skip-USB-devices-when-configuring-DMA-during-probe/20170804-014620 config: microblaze-mmu_defconfig (attached as .config) compiler: microblaze-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=microblaze All errors (new ones prefixed by >>): drivers/base/dma-mapping.o: In function `dma_configure': >> drivers/base/dma-mapping.c:350: undefined reference to `usb_bus_type' drivers/base/dma-mapping.o: In function `dma_deconfigure': drivers/base/dma-mapping.c:377: undefined reference to `usb_bus_type' net/sunrpc/stats.o: In function `rpc_print_iostats': net/sunrpc/stats.c:206: undefined reference to `_GLOBAL_OFFSET_TABLE_' scripts/link-vmlinux.sh: line 93: 76193 Segmentation fault ${LD} ${LDFLAGS} ${LDFLAGS_vmlinux} -o ${2} -T ${lds} ${objects} vim +350 drivers/base/dma-mapping.c 342 343 int dma_configure(struct device *dev) 344 { 345 struct device *bridge = NULL, *dma_dev = dev; 346 enum dev_dma_attr attr; 347 int ret = 0; 348 349 /* USB devices share the controller's mask. */ > 350 if (dev->bus == _bus_type) 351 return 0; 352 353 if (dev_is_pci(dev)) { 354 bridge = pci_get_host_bridge_device(to_pci_dev(dev)); 355 dma_dev = bridge; 356 if (IS_ENABLED(CONFIG_OF) && dma_dev->parent && 357 dma_dev->parent->of_node) 358 dma_dev = dma_dev->parent; 359 } 360 361 if (dma_dev->of_node) { 362 ret = of_dma_configure(dev, dma_dev->of_node); 363 } else if (has_acpi_companion(dma_dev)) { 364 attr = acpi_get_dma_attr(to_acpi_device_node(dma_dev->fwnode)); 365 if (attr != DEV_DMA_NOT_SUPPORTED) 366 ret = acpi_dma_configure(dev, attr); 367 } 368 369 if (bridge) 370 pci_put_host_bridge_device(bridge); 371 372 return ret; 373 } 374 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] dma-mapping: skip USB devices when configuring DMA during probe
On Thu, Aug 03, 2017 at 12:50:20PM +0100, Robin Murphy wrote: > Hi Johan, > > On 03/08/17 11:05, Johan Hovold wrote: > > USB devices use the DMA mask and offset of the controller, which have > > already been setup when a device is probed. Note that modifying the mask > > of a USB device would change the mask for the controller (and all > > devices on the bus) as the mask is literally shared. > > If I understand the situation correctly, some USB devices have to > pretend to be DMA masters in order to interact properly with subsystems > that make DMA mapping calls directly, because those subsystems don't > have access to the 'real' DMA master sysdev (and shouldn't have to know > anyway). If that is the case, then there's really a much more general > problem, because correct DMA API operation can depend on other things > like IOMMU stuff and archdata which cannot be arbitrarily copied or > shared between devices. Right, take a look at usb_alloc_dev() and commit b44bbc46a8bb ("usb: core: setup dma_pfn_offset for USB devices and, interfaces") for some background. Note that we've been setting dma_mask this way since before git. > What with things like the DWC3 glue layer device mess as well, the > feeling I'm getting is that this probably needs addressing at the driver > core/DMA API level - if we had some flag in struct device to say "I can > effectively do DMA, but proxied through my parent", the DMA API could > pick that up and internally chase down the correct DMA master device, > and we could then probably clean up a lot of the "sysdev"-type gunk in > various driver layers as well. > > For expedience of keeping the existing hack working, though, I think > this patch is OK. Yes, we should fix the immediate regression first. > > Since commit 2bf698671205 ("USB: of: fix root-hub device-tree node > > handling"), of_dma_configure() would be called also for root hubs, which > > use the device node of the controller. A separate bug that makes > > of_dma_configure() generate a 30-bit DMA mask from the RPI3's > > "dma-ranges" would thus set a broken mask also for the controller. This > > in turn leads to USB devices failing to enumerate when control transfers > > fail: > > If we're calling out that (long-standing) bug in particular, it might be > worth clarifying that it's benign for the HCD itself because the dwc2 > driver still sets a 32-bit mask afterwards on probe - the breakage is > specifically from the subsequent of_dma_configure() call with the root > hub resetting the HCD's mask again *after* the HCD driver has configured > it (and started to perform DMA), which is just fundamentally wrong > regardless of what it ends up set to. Good point, I'll amend this paragraph. > > dwc2 3f98.usb: Cannot do DMA to address 0x3a166a00 > > > > Fix this, and similar future problems, by simply skipping USB devices > > when dma_configure() is called during probe. > > > > Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for > > platform/amba/pci bus devices") > > Cc: stable# 4.12 > > Cc: Robin Murphy > > Cc: Sricharan R > > Cc: Stefan Wahren > > Reported-by: Hans Verkuil > > Signed-off-by: Johan Hovold > > --- > > drivers/base/dma-mapping.c | 8 > > 1 file changed, 8 insertions(+) > > > > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > > index b555ff9dd8fc..c6cde7e79599 100644 > > --- a/drivers/base/dma-mapping.c > > +++ b/drivers/base/dma-mapping.c > > @@ -13,6 +13,7 @@ > > #include > > #include > > #include > > +#include > > #include > > > > /* > > @@ -345,6 +346,10 @@ int dma_configure(struct device *dev) > > enum dev_dma_attr attr; > > int ret = 0; > > > > + /* USB devices share the controller's mask. */ > > + if (dev->bus == _bus_type) > > Maybe a dev_is_usb() helper to mimic dev_is_pci()? Yeah, I considered that but figured it wasn't essential for a minimal patch marked for stable, but then again, such a macro would be straight-forward enough. I'll respin. > > + return 0; > > + > > if (dev_is_pci(dev)) { > > bridge = pci_get_host_bridge_device(to_pci_dev(dev)); > > dma_dev = bridge; > > @@ -369,6 +374,9 @@ int dma_configure(struct device *dev) > > > > void dma_deconfigure(struct device *dev) > > { > > + if (dev->bus == _bus_type) > > + return; > > + > > of_dma_deconfigure(dev); > > acpi_dma_deconfigure(dev); > > } Thanks, Johan -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] dma-mapping: skip USB devices when configuring DMA during probe
Hi Johan, On 03/08/17 11:05, Johan Hovold wrote: > USB devices use the DMA mask and offset of the controller, which have > already been setup when a device is probed. Note that modifying the mask > of a USB device would change the mask for the controller (and all > devices on the bus) as the mask is literally shared. If I understand the situation correctly, some USB devices have to pretend to be DMA masters in order to interact properly with subsystems that make DMA mapping calls directly, because those subsystems don't have access to the 'real' DMA master sysdev (and shouldn't have to know anyway). If that is the case, then there's really a much more general problem, because correct DMA API operation can depend on other things like IOMMU stuff and archdata which cannot be arbitrarily copied or shared between devices. What with things like the DWC3 glue layer device mess as well, the feeling I'm getting is that this probably needs addressing at the driver core/DMA API level - if we had some flag in struct device to say "I can effectively do DMA, but proxied through my parent", the DMA API could pick that up and internally chase down the correct DMA master device, and we could then probably clean up a lot of the "sysdev"-type gunk in various driver layers as well. For expedience of keeping the existing hack working, though, I think this patch is OK. > Since commit 2bf698671205 ("USB: of: fix root-hub device-tree node > handling"), of_dma_configure() would be called also for root hubs, which > use the device node of the controller. A separate bug that makes > of_dma_configure() generate a 30-bit DMA mask from the RPI3's > "dma-ranges" would thus set a broken mask also for the controller. This > in turn leads to USB devices failing to enumerate when control transfers > fail: If we're calling out that (long-standing) bug in particular, it might be worth clarifying that it's benign for the HCD itself because the dwc2 driver still sets a 32-bit mask afterwards on probe - the breakage is specifically from the subsequent of_dma_configure() call with the root hub resetting the HCD's mask again *after* the HCD driver has configured it (and started to perform DMA), which is just fundamentally wrong regardless of what it ends up set to. > dwc2 3f98.usb: Cannot do DMA to address 0x3a166a00 > > Fix this, and similar future problems, by simply skipping USB devices > when dma_configure() is called during probe. > > Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for > platform/amba/pci bus devices") > Cc: stable# 4.12 > Cc: Robin Murphy > Cc: Sricharan R > Cc: Stefan Wahren > Reported-by: Hans Verkuil > Signed-off-by: Johan Hovold > --- > drivers/base/dma-mapping.c | 8 > 1 file changed, 8 insertions(+) > > diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c > index b555ff9dd8fc..c6cde7e79599 100644 > --- a/drivers/base/dma-mapping.c > +++ b/drivers/base/dma-mapping.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > #include > > /* > @@ -345,6 +346,10 @@ int dma_configure(struct device *dev) > enum dev_dma_attr attr; > int ret = 0; > > + /* USB devices share the controller's mask. */ > + if (dev->bus == _bus_type) Maybe a dev_is_usb() helper to mimic dev_is_pci()? Robin. > + return 0; > + > if (dev_is_pci(dev)) { > bridge = pci_get_host_bridge_device(to_pci_dev(dev)); > dma_dev = bridge; > @@ -369,6 +374,9 @@ int dma_configure(struct device *dev) > > void dma_deconfigure(struct device *dev) > { > + if (dev->bus == _bus_type) > + return; > + > of_dma_deconfigure(dev); > acpi_dma_deconfigure(dev); > } > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] dma-mapping: skip USB devices when configuring DMA during probe
USB devices use the DMA mask and offset of the controller, which have already been setup when a device is probed. Note that modifying the mask of a USB device would change the mask for the controller (and all devices on the bus) as the mask is literally shared. Since commit 2bf698671205 ("USB: of: fix root-hub device-tree node handling"), of_dma_configure() would be called also for root hubs, which use the device node of the controller. A separate bug that makes of_dma_configure() generate a 30-bit DMA mask from the RPI3's "dma-ranges" would thus set a broken mask also for the controller. This in turn leads to USB devices failing to enumerate when control transfers fail: dwc2 3f98.usb: Cannot do DMA to address 0x3a166a00 Fix this, and similar future problems, by simply skipping USB devices when dma_configure() is called during probe. Fixes: 09515ef5ddad ("of/acpi: Configure dma operations at probe time for platform/amba/pci bus devices") Cc: stable# 4.12 Cc: Robin Murphy Cc: Sricharan R Cc: Stefan Wahren Reported-by: Hans Verkuil Signed-off-by: Johan Hovold --- drivers/base/dma-mapping.c | 8 1 file changed, 8 insertions(+) diff --git a/drivers/base/dma-mapping.c b/drivers/base/dma-mapping.c index b555ff9dd8fc..c6cde7e79599 100644 --- a/drivers/base/dma-mapping.c +++ b/drivers/base/dma-mapping.c @@ -13,6 +13,7 @@ #include #include #include +#include #include /* @@ -345,6 +346,10 @@ int dma_configure(struct device *dev) enum dev_dma_attr attr; int ret = 0; + /* USB devices share the controller's mask. */ + if (dev->bus == _bus_type) + return 0; + if (dev_is_pci(dev)) { bridge = pci_get_host_bridge_device(to_pci_dev(dev)); dma_dev = bridge; @@ -369,6 +374,9 @@ int dma_configure(struct device *dev) void dma_deconfigure(struct device *dev) { + if (dev->bus == _bus_type) + return; + of_dma_deconfigure(dev); acpi_dma_deconfigure(dev); } -- 2.13.3 -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html