Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-17 Thread Fredrik Noring
Hi Laurentiu,

> > that I tracked down to the calls
> > 
> >hub_port_init
> > -> usb_control_msg
> > -> usb_internal_control_msg
> > -> usb_start_wait_urb
> > -> usb_submit_urb
> > -> usb_hcd_submit_urb
> > -> hcd->driver->urb_enqueue
> > -> ohci_urb_enqueue
> > -> ed_get
> > -> ed_alloc

I found that the generic OHCI driver takes a wrong turn here, in ed_alloc,
and eventually also in td_alloc. Fortunately, your patch can be easily
extended to fix them as well. Please see attached patch below.

With that, the OHCI seems to work as expected with HCD_LOCAL_MEM. :)

Would you like me to submit gen_pool_dma_zalloc as a separate patch?

> That looks indeed worrisome but I'd expect that it's not related to 
> these genalloc changes that I'm working on. I'll try to look into it but 
> as I'm not familiar with the area maybe others will comment.

It seems appropriate to verify that the IRQ will not end up sleeping
somewhere in the pool.

> > Also, DMA_BUFFER_SIZE in ohci-ps2.c is only 256 KiB in total. Is the new
> > pool implementation at least as efficient as the previous one?
> 
> I don't see any obvious reasons for genalloc to be less efficient.

OK, good.

> One thing I noticed between genalloc and the original implementation is 
> that previously the device memory was mapped with memremap() while with 
> genalloc I'm doing a devm_ioremap(). I can't think of a relevant 
> difference except that memremap() maps with WC buth maybe there are 
> other arch specific subtleties. I've attached a new version of the 
> ohci-ps2 patch replacing the devm_ioremap() with devm_memremap() to 
> better match the original implementation. Please test if you find some time.

You're right, it makes no difference.

Fredrik

diff --git a/drivers/usb/host/ohci-mem.c b/drivers/usb/host/ohci-mem.c
--- a/drivers/usb/host/ohci-mem.c
+++ b/drivers/usb/host/ohci-mem.c
@@ -82,10 +82,14 @@ dma_to_td (struct ohci_hcd *hc, dma_addr_t td_dma)
 static struct td *
 td_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
 {
+   struct usb_hcd  *hcd = ohci_to_hcd(hc);
dma_addr_t  dma;
struct td   *td;
 
-   td = dma_pool_zalloc (hc->td_cache, mem_flags, &dma);
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   td = gen_pool_dma_zalloc (hcd->localmem_pool, sizeof(*td), 
&dma);
+   else
+   td = dma_pool_zalloc (hc->td_cache, mem_flags, &dma);
if (td) {
/* in case hc fetches it, make it look dead */
td->hwNextTD = cpu_to_hc32 (hc, dma);
@@ -98,6 +102,7 @@ td_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
 static void
 td_free (struct ohci_hcd *hc, struct td *td)
 {
+   struct usb_hcd  *hcd = ohci_to_hcd(hc);
struct td   **prev = &hc->td_hash [TD_HASH_FUNC (td->td_dma)];
 
while (*prev && *prev != td)
@@ -106,7 +111,11 @@ td_free (struct ohci_hcd *hc, struct td *td)
*prev = td->td_hash;
else if ((td->hwINFO & cpu_to_hc32(hc, TD_DONE)) != 0)
ohci_dbg (hc, "no hash for td %p\n", td);
-   dma_pool_free (hc->td_cache, td, td->td_dma);
+
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   gen_pool_free (hcd->localmem_pool, (unsigned long)td, 
sizeof(*td));
+   else
+   dma_pool_free (hc->td_cache, td, td->td_dma);
 }
 
 /*-*/
@@ -115,10 +124,14 @@ td_free (struct ohci_hcd *hc, struct td *td)
 static struct ed *
 ed_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
 {
+   struct usb_hcd  *hcd = ohci_to_hcd(hc);
dma_addr_t  dma;
struct ed   *ed;
 
-   ed = dma_pool_zalloc (hc->ed_cache, mem_flags, &dma);
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   ed = gen_pool_dma_zalloc (hcd->localmem_pool, sizeof(*ed), 
&dma);
+   else
+   ed = dma_pool_zalloc (hc->ed_cache, mem_flags, &dma);
if (ed) {
INIT_LIST_HEAD (&ed->td_list);
ed->dma = dma;
@@ -129,6 +142,11 @@ ed_alloc (struct ohci_hcd *hc, gfp_t mem_flags)
 static void
 ed_free (struct ohci_hcd *hc, struct ed *ed)
 {
-   dma_pool_free (hc->ed_cache, ed, ed->dma);
+   struct usb_hcd  *hcd = ohci_to_hcd(hc);
+
+   if (hcd->driver->flags & HCD_LOCAL_MEM)
+   gen_pool_free (hcd->localmem_pool, (unsigned long)ed, 
sizeof(*ed));
+   else
+   dma_pool_free (hc->ed_cache, ed, ed->dma);
 }
 
diff --git a/include/linux/genalloc.h b/include/linux/genalloc.h
--- a/include/linux/genalloc.h
+++ b/include/linux/genalloc.h
@@ -121,6 +121,8 @@ extern unsigned long gen_pool_alloc_algo(struct gen_pool *, 
size_t,
genpool_algo_t algo, void *data);
 extern void *gen_pool_dma_alloc(struct gen_pool *pool, size_t size,
dma_addr_t *dma);
+extern void *gen_pool_dma_zalloc(struct gen_pool *pool, size_t size,
+   dma_addr_t *dma);
 extern void gen_pool_free(

Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Fredrik Noring
Hi Laurentiu,

> I took your code, added the missing mapping and placed it in a patch. 
> Please see attached (compile tested only).

Thanks! Unfortunately, the OHCI fails with errors such as

usb 1-1: device descriptor read/64, error -12

that I tracked down to the calls

   hub_port_init
-> usb_control_msg
-> usb_internal_control_msg
-> usb_start_wait_urb
-> usb_submit_urb
-> usb_hcd_submit_urb
-> hcd->driver->urb_enqueue
-> ohci_urb_enqueue
-> ed_get
-> ed_alloc
-> dma_pool_zalloc
-> dma_pool_alloc
-> pool_alloc_page,

which returns NULL. Then I noticed

/* pool_alloc_page() might sleep, so temporarily drop &pool->lock */

that might be a problem considering that the HCD handles pool memory in
IRQ handlers, for instance:

   do_IRQ
-> generic_handle_irq
-> handle_level_irq
-> handle_irq_event
-> handle_irq_event_percpu
-> __handle_irq_event_percpu
-> usb_hcd_irq
-> ohci_irq
-> ohci_work
-> finish_urb
-> __usb_hcd_giveback_urb
-> usb_hcd_unmap_urb_for_dma
-> hcd_buffer_free

Also, DMA_BUFFER_SIZE in ohci-ps2.c is only 256 KiB in total. Is the new
pool implementation at least as efficient as the previous one?

Fredrik


Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-16 Thread Fredrik Noring
Hi Laurentiu,

> > The kernel oopses with "unable to handle kernel paging request at virtual
> > address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. 
> 
> By any chance, does this address looks like the dma_addr that the device 
> should target?

Yes, that looks like a typical device address suitable for its DMA.

> Actually, I think I'm misusing genalloc and also it appears that i need 
> to add a mapping on the phys address. So my plan is to change the 
> "unsigned long virt" to be the void * returned by the mapping operation 
> and the phys_addr_t be the dma_addr_t. I'll return with a patch.

Great, I'm happy to test your next patch revision.

Fredrik


Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-15 Thread Fredrik Noring
Hi Lauretniu,

> I think that any recent kernel will do, so I'd say your current branch 
> should be fine.

The kernel oopses with "unable to handle kernel paging request at virtual
address 000aba0b" in hcd_alloc_coherent via usb_hcd_map_urb_for_dma. This
relates to patch 2/3 that I didn't quite understand, where

-   retval = dma_declare_coherent_memory(dev, mem->start,
-mem->start - mem->parent->start,
-resource_size(mem));

becomes

+   retval = gen_pool_add_virt(hcd->localmem_pool,
+  (unsigned long)mem->start -
+   mem->parent->start,
+  mem->start, resource_size(mem),

so that arguments two and three switch places. Given

int dma_declare_coherent_memory(struct device *dev, phys_addr_t phys_addr,
dma_addr_t device_addr, size_t size);

and

int gen_pool_add_virt(struct gen_pool *pool, unsigned long virt, phys_addr_t 
phys
 size_t size, int nid)

it seems that the device address (dma_addr_t device_addr) now becomes a
virtual address (unsigned long virt). Is that intended?

My corresponding patch is below for reference. I applied your 1/3 patch
to test it.

Fredrik

diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
--- a/drivers/usb/host/ohci-ps2.c
+++ b/drivers/usb/host/ohci-ps2.c
@@ -7,6 +7,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -92,12 +93,12 @@ static irqreturn_t ohci_ps2_irq(struct usb_hcd *hcd)
return ohci_irq(hcd); /* Call normal IRQ handler. */
 }
 
-static int iopheap_alloc_coherent(struct platform_device *pdev,
-   size_t size, int flags)
+static int iopheap_alloc_coherent(struct platform_device *pdev, size_t size)
 {
struct device *dev = &pdev->dev;
struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
+   int err;
 
if (ps2priv->iop_dma_addr != 0)
return 0;
@@ -112,28 +113,37 @@ static int iopheap_alloc_coherent(struct platform_device 
*pdev,
return -ENOMEM;
}
 
-   if (dma_declare_coherent_memory(dev,
-   iop_bus_to_phys(ps2priv->iop_dma_addr),
-   ps2priv->iop_dma_addr, size, flags)) {
-   dev_err(dev, "dma_declare_coherent_memory failed\n");
-   iop_free(ps2priv->iop_dma_addr);
-   ps2priv->iop_dma_addr = 0;
-   return -ENOMEM;
+   hcd->localmem_pool = devm_gen_pool_create(dev,
+   PAGE_SHIFT, dev_to_node(dev), DRV_NAME);
+   if (IS_ERR(hcd->localmem_pool)) {
+   err = PTR_ERR(hcd->localmem_pool);
+   goto out_err;
+   }
+
+   err = gen_pool_add_virt(hcd->localmem_pool, ps2priv->iop_dma_addr,
+   iop_bus_to_phys(ps2priv->iop_dma_addr), size, dev_to_node(dev));
+   if (err < 0) {
+   dev_err(dev, "gen_pool_add_virt failed with %d\n", err);
+   goto out_err;
}
 
return 0;
+
+out_err:
+   iop_free(ps2priv->iop_dma_addr);
+   ps2priv->iop_dma_addr = 0;
+
+   return err;
 }
 
 static void iopheap_free_coherent(struct platform_device *pdev)
 {
-   struct device *dev = &pdev->dev;
struct usb_hcd *hcd = platform_get_drvdata(pdev);
struct ps2_hcd *ps2priv = hcd_to_priv(hcd);
 
if (ps2priv->iop_dma_addr == 0)
return;
 
-   dma_release_declared_memory(dev);
iop_free(ps2priv->iop_dma_addr);
ps2priv->iop_dma_addr = 0;
 }
@@ -177,8 +187,7 @@ static int ohci_hcd_ps2_probe(struct platform_device *pdev)
goto err;
}
 
-   ret = iopheap_alloc_coherent(pdev,
-   DMA_BUFFER_SIZE, DMA_MEMORY_EXCLUSIVE);
+   ret = iopheap_alloc_coherent(pdev, DMA_BUFFER_SIZE);
if (ret != 0)
goto err_alloc_coherent;
 


Re: [RFC PATCH v2 0/3] prerequisites for device reserved local mem rework

2019-05-14 Thread Fredrik Noring
Thanks Robin!

> > For HCs that have local memory, replace the current DMA API usage
> > with a genalloc generic allocator to manage the mappings for these
> > devices.
> > This is in preparation for dropping the existing "coherent" dma
> > mem declaration APIs. Current implementation was relying on a short
> > circuit in the DMA API that in the end, was acting as an allocator
> > for these type of devices.
> > 
> > Only compiled tested, so any volunteers willing to test are most welcome.
> 
> I recall an out-of-tree PlayStation 2 OHCI driver being another 
> HCD_LOCAL_MEM user - if Fredrik and Juergen are still active on that, 
> hopefully they might be able to comment on whether this approach can 
> work for them too. Patchwork link just in case: 
> https://patchwork.kernel.org/project/linux-usb/list/?series=117433

True. In fact I'm preparing a patch submission for this PS2 OHCI driver,
along with about a hundred other patches (unrelated to the USB subsystem).
Hopefully in a few weeks. My patches are currently on top of v5.0. What
branch/version is recommended to try this DMA update?

Here is the v5.0.11 PS2 OHCI driver, for reference:

https://github.com/frno7/linux/blob/ps2-v5.0.11/drivers/usb/host/ohci-ps2.c

Fredrik


Re: [RFC v3] Add support for Sony PS2 OHCI

2018-06-24 Thread Fredrik Noring
Hi Alan Stern,

On Wed, Mar 14, 2018 at 04:52:13PM -0400, Alan Stern wrote:
> This should not be needed; it indicates that something is wrong with 
> the way interrupt requests are handled on this platform.
> 
> Are you using threaded interrupts at all?  If you are, you have to 
> change the spin_lock() call in ohci_irq() to spin_lock_irqsave().  You 
> could try making this change in any case, to see if it prevents the 
> freezes.

No, I'm not using threaded interrupts, so spin_lock_irqsave() did
unfortunately not help. However, replacing the OHCI interrupt with a
1000 Hz timer works perfectly. Perhaps the problem is that a wanted
interrupt is lost somehow?

Would you recommend any method to trace the OHCI to investigate its
state at the freezing point? It's a MIPS architecture and v4.17.

Fredrik
--
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 05/22] USB: mon: no need to check return value of debugfs_create functions

2018-05-29 Thread Fredrik Noring
Hi Greg,

On Tue, May 29, 2018 at 06:51:54PM +0200, Greg Kroah-Hartman wrote:
> On Tue, May 29, 2018 at 11:34:34AM -0500, Pete Zaitcev wrote:
> > On Tue, 29 May 2018 17:30:50 +0200
> > Greg Kroah-Hartman  wrote:
> > 
> > > When calling debugfs functions, there is no need to ever check the
> > > return value.  The function can work or not, but the code logic should
> > > never do something different based on this.
> > 
> > Okay, fair enough. And the code works, surprisingly, even when the
> > debugfs is disabled (well, according to my review). I just have one
> > question - wouldn't it be cleaner to deprecate and remove the text
> > API altogether?
> 
> If you think we can do that, sure!  But what's the odds that someone
> still uses it?  This patch doesn't change any real functionality.

I had some use of this text API last year, when I also discovered and
submitted and fix for its data corruption problems described in commit
a5f596830e27e "usb: usbmon: Read text within supplied buffer size".

A significant disadvantage with the text API is, as far as I understand it,
that it can drop events (due to its buffering limit) without informing API
readers (via event sequence numbers or some such).

I've found a couple of USB bugs/problems (commit d6c931ea32dc0 "USB: OHCI:
Fix NULL dereference in HCDs using HCD_LOCAL_MEM" for instance) and I'm
still investigating at least one bug possibly related to lost interrupts
where some kind of probing method could be helpful.

However, I would explore other kernel probing methods for that. Not only
because I'm curious but also because I have the impression that there are
more powerful methods available than this text API. Would you recommend
any alternatives?

Fredrik
--
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: [RFC v3] Add support for Sony PS2 OHCI

2018-03-14 Thread Fredrik Noring
Hi Alan Stern,

> > > What happened to the changes to ohci_run() and ohci_irq() that were in 
> > > v2 of this RFC?  Did you decide they weren't needed?
> > 
> > In v3 Jürgen Urban folded those changes into the driver functions
> > 
> > ohci_ps2_enable(),
> > ohci_ps2_disable(),
> > ohci_ps2_start_hc() and
> > ohci_ps2_stop_hc()
> > 
> > in drivers/usb/host/ohci-ps2.c.
> 
> Good, I was going to suggest doing that.

There is one quirk with the ISR that perhaps could be clarified: For some
reason we have to intercept ohci_irq to disable interrupts, as shown in the
function below, otherwise the whole USB subsystem eventually will freeze up.

Is this a known OHCI problem? Is some part of its USB core ISR not reentrant?

static irqreturn_t ohci_ps2_irq(struct usb_hcd *hcd)
{
struct ohci_hcd *ohci = hcd_to_ohci(hcd);
struct ohci_regs __iomem *regs = ohci->regs;

/*
 * FIXME: For some reason OHCI_INTR_MIE is required in the
 * IRQ handler. Without it, reading a large amount of data
 * (> 1 GB) from a mass storage device results in a freeze.
 */
ohci_writel(ohci, OHCI_INTR_MIE, ®s->intrdisable);

return ohci_irq(hcd); /* Call normal IRQ handler. */
}

Fredrik
--
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: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-11 Thread Fredrik Noring
Hi Christoph,

> The point is that you should always use a pool, period.
> dma_alloc*/dma_free* are fundamentally expensive operations on my
> architectures, so if you call them from a fast path you are doing
> something wrong.

The author's comment in commit b3476675320e "usb: dma bounce buffer support"
seems to suggest that the greatest concern is space efficiency as opposed to
speed. I tried to evaluate strict pool allocations, similar to the patch
below, but that didn't turn out as I expected.

I chose a 64 KiB pool maximum since it has been the largest requested size I
have observed in USB traces (which may not hold in general, of course). This
change caused the USB mass storage driver to get stuck in some kind of memory
deadlock, with endless busy-looping on 64 KiB allocation failures.

I also tried a progression of pool sizes including nonpowers of two, for
example 12288, to make better use of the 256 KiB memory capacity. However,
the following part of dma_pool_create in linux/mm/dmapool.c is somewhat
puzzling:

if (!boundary)
boundary = allocation;
else if ((boundary < size) || (boundary & (boundary - 1)))
return NULL;

Is the boundary variable required to be a power of two only when it is
explicitly given as nonzero?

Fredrik

diff --git a/drivers/usb/core/buffer.c b/drivers/usb/core/buffer.c
index 77eef8acff94..8cc8fbc91c76 100644
--- a/drivers/usb/core/buffer.c
+++ b/drivers/usb/core/buffer.c
@@ -26,7 +26,7 @@
 
 /* FIXME tune these based on pool statistics ... */
 static size_t pool_max[HCD_BUFFER_POOLS] = {
-   32, 128, 512, 2048,
+   32, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536
 };
 
 void __init usb_init_pool_max(void)
@@ -76,7 +76,7 @@ int hcd_buffer_create(struct usb_hcd *hcd)
continue;
snprintf(name, sizeof(name), "buffer-%d", size);
hcd->pool[i] = dma_pool_create(name, hcd->self.sysdev,
-   size, size, 0);
+   size, min_t(size_t, 4096, size), 0);
if (!hcd->pool[i]) {
hcd_buffer_destroy(hcd);
return -ENOMEM;
@@ -140,7 +140,7 @@ void *hcd_buffer_alloc(
if (size <= pool_max[i])
return dma_pool_alloc(hcd->pool[i], mem_flags, dma);
}
-   return dma_alloc_coherent(hcd->self.sysdev, size, dma, mem_flags);
+   return NULL;
 }
 
 void hcd_buffer_free(
@@ -169,5 +169,5 @@ void hcd_buffer_free(
return;
}
}
-   dma_free_coherent(hcd->self.sysdev, size, addr, dma);
+   BUG();  /* The buffer could not have been allocated. */
 }
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index 176900528822..2075f1e22e32 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -189,7 +189,7 @@ struct usb_hcd {
struct usb_hcd  *primary_hcd;
 
 
-#define HCD_BUFFER_POOLS   4
+#define HCD_BUFFER_POOLS   11
struct dma_pool *pool[HCD_BUFFER_POOLS];
 
int state;
--
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] USB: OHCI: Fix NULL dereference in HCDs using HCD_LOCAL_MEM

2018-03-09 Thread Fredrik Noring
Scatter-gather needs to be disabled when using dma_declare_coherent_memory
and HCD_LOCAL_MEM. Andrea Righi made the equivalent fix for EHCI drivers
in commit 4307a28eb01284 "USB: EHCI: fix NULL pointer dererence in HCDs
that use HCD_LOCAL_MEM".

The following NULL pointer WARN_ON_ONCE triggered with OHCI drivers:

[ cut here ]
WARNING: CPU: 0 PID: 49 at drivers/usb/core/hcd.c:1379 
hcd_alloc_coherent+0x4c/0xc8
Modules linked in:
CPU: 0 PID: 49 Comm: usb-storage Not tainted 4.15.0+ #1014
Stack :   805a78d2 003a 81f5c2cc 8053d367 804d77fc 0031
805a3a08 0563 81ee9400 805a  10058c00 81f61b10 805c
  805a 00d9038e 0004 803ee818 0006 312e3420
805c  0073 81f61958   802eb380 804fd538
0009 0563 81ee9400 805a 0002 80056148  805a
...
Call Trace:
[<578af360>] show_stack+0x74/0x104
[<2f3702c6>] __warn+0x118/0x120
[] warn_slowpath_null+0x44/0x58
[] hcd_alloc_coherent+0x4c/0xc8
[<3578fa36>] usb_hcd_map_urb_for_dma+0x4d8/0x534
[<110bc94c>] usb_hcd_submit_urb+0x82c/0x834
[<02eb5baf>] usb_sg_wait+0x14c/0x1a0
[] usb_stor_bulk_transfer_sglist.part.1+0xac/0x124
[<87a5c34c>] usb_stor_bulk_srb+0x40/0x60
[] usb_stor_Bulk_transport+0x160/0x37c
[] usb_stor_invoke_transport+0x3c/0x500
[<004754f4>] usb_stor_control_thread+0x258/0x28c
[<22edf42e>] kthread+0x134/0x13c
[] ret_from_kernel_thread+0x14/0x1c
---[ end trace bcdb825805eefdcc ]---

Signed-off-by: Fredrik Noring 

diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -446,7 +446,8 @@ static int ohci_init (struct ohci_hcd *ohci)
struct usb_hcd *hcd = ohci_to_hcd(ohci);
 
/* Accept arbitrarily long scatter-gather lists */
-   hcd->self.sg_tablesize = ~0;
+   if (!(hcd->driver->flags & HCD_LOCAL_MEM))
+   hcd->self.sg_tablesize = ~0;
 
if (distrust_firmware)
ohci->flags |= OHCI_QUIRK_HUB_POWER;
--
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: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-04 Thread Fredrik Noring
Hi Alan Stern,

> > Alan, can dma_free_coherent be delayed to a point when IRQs are enabled?
> 
> Yes, subject to the usual concerns about not being delayed for too 
> long.  Also, some HCDs are highly memory-constrained.  I don't know if 
> they use this API, but if they do then delaying a free could result in 
> not enough memory being available when an allocation is needed.

The PS2 HCD in this case is limited to 256 KiB and memory allocation
failures can be observed frequently in USB traces (102 subsequent failures
in the trace below involving mass storage transactions, for example). Is
there any smartness to the allocations? My impression is that the USB core
loops until it gets what it wants, and then happily resumes. Does it busy
wait?

The RT3070 wireless USB device driver, for example, has hardcoded buffer
limits that exceed the total amount of available memory. It refuses to
accept devices unless adjusted in the source (as in the patch below), after
which it works nicely.

Other USB device drivers such as the one for the AR9271 wireless device
trigger endlessly repeating kernel warnings claiming

BOGUS urb xfer, pipe 1 != type 3

as shown in this kernel backtrace:

[ cut here ]
WARNING: CPU: 0 PID: 15 at drivers/usb/core/urb.c:471 usb_submit_urb+0x280/0x528
usb 1-1: BOGUS urb xfer, pipe 1 != type 3
Modules linked in: ath9k_htc ath9k_common ath9k_hw ath mac80211 cfg80211 usbmon
CPU: 0 PID: 15 Comm: kworker/0:1 Tainted: GW4.15.0+ #831
Workqueue: events request_firmware_work_func
Stack : 0009 01d7 0001 81714028 81714000 8006161c 80519af4 000f
805e3b08 01d7 80519a68 81cd3ad4 81714000 10058c00 81cd3aa8 80587340
  03f8 0007  03f8  
0040 0008  81ccd288   803036a8 8053e854
0009 01d7 0001 81714028 0008 ffc7  805e
...
Call Trace:
[<2318fb9b>] show_stack+0x74/0x104
[] __warn+0x118/0x120
[<01359d10>] warn_slowpath_fmt+0x30/0x3c
[] usb_submit_urb+0x280/0x528
[<89eb7b59>] hif_usb_send+0x3b0/0x3e0 [ath9k_htc]
[<51446a9c>] ath9k_wmi_cmd+0x194/0x228 [ath9k_htc]
[<2119badc>] ath9k_regread+0x5c/0x88 [ath9k_htc]
[] ath9k_hw_wait+0xa4/0xc0 [ath9k_hw]
[<368b3c01>] ath9k_hw_set_reset_reg+0x23c/0x288 [ath9k_hw]
[<28807ea6>] ath9k_hw_init+0x3e8/0xa24 [ath9k_hw]
[] ath9k_htc_probe_device+0x38c/0xa2c [ath9k_htc]
[] ath9k_htc_hw_init+0x20/0x4c [ath9k_htc]
[<6500cd86>] ath9k_hif_usb_firmware_cb+0x780/0x854 [ath9k_htc]
[] request_firmware_work_func+0x40/0x70
[<54f8834a>] process_one_work+0x1f4/0x358
[<23b84d12>] worker_thread+0x354/0x4b8
[<583bf61b>] kthread+0x134/0x13c
[<213b5c9e>] ret_from_kernel_thread+0x14/0x1c
---[ end trace 2a97c69dce30b650 ]---

I don't know if this is related to memory limitations or some other problem
though.

Fredrik

--- a/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
+++ b/drivers/net/wireless/ralink/rt2x00/rt2800usb.c
@@ -871,7 +871,7 @@ static void rt2800usb_queue_init(struct data_queue *queue)
 
switch (queue->qid) {
case QID_RX:
-   queue->limit = 128;
+   queue->limit = 8;
queue->data_size = AGGREGATION_SIZE;
queue->desc_size = RXINFO_DESC_SIZE;
queue->winfo_size = rxwi_size;
@@ -882,7 +882,7 @@ static void rt2800usb_queue_init(struct data_queue *queue)
case QID_AC_VI:
case QID_AC_BE:
case QID_AC_BK:
-   queue->limit = 16;
+   queue->limit = 8;
queue->data_size = AGGREGATION_SIZE;
queue->desc_size = TXINFO_DESC_SIZE;
queue->winfo_size = txwi_size;

...
81f58280 299082198 C Bi:1:003:1 0 13 = 55534253 ...
81f58280 299082510 S Bo:1:003:2 -150 31 = 55534243 ...
81f58280 299083200 C Bo:1:003:2 0 31 >
81f9aa00 299083298 S Bi:1:003:1 -150 65536 <
81f9a800 299093849 S Ci:1:002:0 s c0 07  1700 0004 4 <
81f9ab00 299095041 S Bi:1:003:1 -150 8192 <
81f9a800 299095229 C Ci:1:002:0 0 4 = 2d00
81f9a680 299096808 S Bi:1:003:1 -150 49152 <
81f9a680 299096838 E Bi:1:003:1 -12 0
81f9a680 299097868 S Bi:1:003:1 -150 49152 <
81f9a680 299097896 E Bi:1:003:1 -12 0
81f9a680 299098894 S Bi:1:003:1 -150 49152 <
... 97 allocation failures repeated ...
81f9a680 299151156 S Bi:1:003:1 -150 49152 <
81f9a680 299151166 E Bi:1:003:1 -12 0
81f9a680 299151179 S Bi:1:003:1 -150 49152 <
81f9a680 299151190 E Bi:1:003:1 -12 0
81f9a680 299151203 S Bi:1:003:1 -150 49152 <
81f9a680 299151216 E Bi:1:003:1 -12 0
81f9a680 299151231 S Bi:1:003:1 -150 49152 <
81f9a680 299159909 S Bi:1:003:1 -150 49152 <
81f9a680 299160814 S Bi:1:003:1 -150 49152 <
81f9a680 299161732 S Bi:1:003:1 -150 49152 <
81f9a680 299162644 S Bi:1:003:1 -150 49152 <
81f9a680 299163574 S Bi:1:003:1 -150 49152 <
81f9a680 299164490 S Bi:1:003:1 -150 49152 <
81f9aa00 299172593 C Bi:1:003:1 0 65536 = f7b07b39 ...
81f9ab00 299175259 C Bi:1:003:1 0 8192 = 26e2c254 ..

Re: WARN_ON(irqs_disabled()) in dma_free_attrs?

2018-03-03 Thread Fredrik Noring
Christoph, Alan,

> If it is allocating / freeing this memory all the time in the hot path
> it should really use a dma pool (see include/ilinux/dmapool.h).
> The dma coherent APIs aren't really built for being called in the
> hot path.

hcd_buffer_free uses a combination of dma pools and dma coherent APIs:

...
for (i = 0; i < HCD_BUFFER_POOLS; i++) {
if (size <= pool_max[i]) {
dma_pool_free(hcd->pool[i], addr, dma);
return;
}
}
dma_free_coherent(hcd->self.sysdev, size, addr, dma);

Alan, can dma_free_coherent be delayed to a point when IRQs are enabled?

[ Links to previous messages on this topic are listed below. ]

Fredrik

https://www.spinics.net/lists/linux-usb/msg162817.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026334.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026335.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026336.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026337.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026338.html
https://lists.linuxfoundation.org/pipermail/iommu/2018-March/026339.html
--
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: [RFC v3] Add support for Sony PS2 OHCI

2018-02-01 Thread Fredrik Noring
Hi Alan Stern,

On Mon, Nov 27, 2017 at 10:46:55AM -0500, Alan Stern wrote:
> I did not read the whole thing in detail, but it generally looks okay.
> Except, of course, that the dma-mapping.h change can't be part of this 
> patch.  That will have to be done separately.

We are observing seemingly random kernel crashes and failures with mass
storage devices and "Unable to allocate URBs" with ath9k_htc.
hcd_buffer_alloc occasionally fails (returns NULL). Considering that only
256 KiB is reserved with dma_declare_coherent_memory, would you recommend
using any additional facilities for example bounce buffers or some such to
handle the memory requirements for USB device drivers?

Kernel crashes appear more frequently when using usbmon too.

Fredrik
--
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: [RFC v3] Add support for Sony PS2 OHCI

2017-11-29 Thread Fredrik Noring
Hi Alan Stern,

> I did not read the whole thing in detail, but it generally looks okay.
> Except, of course, that the dma-mapping.h change can't be part of this 
> patch.  That will have to be done separately.

Agreed.

> What happened to the changes to ohci_run() and ohci_irq() that were in 
> v2 of this RFC?  Did you decide they weren't needed?

In v3 Jürgen Urban folded those changes into the driver functions

ohci_ps2_enable(),
ohci_ps2_disable(),
ohci_ps2_start_hc() and
ohci_ps2_stop_hc()

in drivers/usb/host/ohci-ps2.c.

Fredrik
--
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: [RFC v3] Add support for Sony PS2 OHCI

2017-11-26 Thread Fredrik Noring
Hi Alan Stern,

> > Be aware that your driver should utilize ohci_init_driver(), like
> > several of the other platform-specific OHCI drivers do.  Unless there's
> > some very good reason, new drivers should never use the old interface.
> 
> Agreed, please find updated patch with the new interface. (I suppose the
> changes to drivers/usb/host/ohci-hcd.c eventually will have to be clarified
> and moved elsewhere too.)

The original author Jürgen Urban has made several improvements to v3 of the
driver, shown below. To make progress we had to remove this WARN_ON line:

--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -527,7 +527,6 @@ static inline void dma_free_attrs(struct device *dev, 
size_t size,
const struct dma_map_ops *ops = get_dma_ops(dev);
 
BUG_ON(!ops);
-   WARN_ON(irqs_disabled());
 
if (dma_release_from_dev_coherent(dev, get_order(size), cpu_addr))
return;

We also tried a change along the lines of

--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -444,8 +444,13 @@ static int ohci_init (struct ohci_hcd *ohci)
int ret;
struct usb_hcd *hcd = ohci_to_hcd(ohci);
 
-   /* Accept arbitrarily long scatter-gather lists */
-   hcd->self.sg_tablesize = ~0;
+   if (hcd->self.uses_dma) {
+   /* Accept arbitrarily long scatter-gather lists */
+   hcd->self.sg_tablesize = ~0;
+   } else {
+   /* DMA not supported. */
+   hcd->self.sg_tablesize = 0;
+   }
 
if (distrust_firmware)
ohci->flags |= OHCI_QUIRK_HUB_POWER;

to address the issue of scatter-gather in combination with HCD_LOCAL_MEM and
dma_declare_coherent_memory().

The PS2 kernel is currently somewhat unstable, but it is at the moment unclear
whether this is due to its OHCI driver.

What are your thoughts on these changes and the driver patch below?

Fredrik

diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -571,6 +571,13 @@ config USB_OHCI_EXYNOS
help
 Enable support for the Samsung Exynos SOC's on-chip OHCI controller.
 
+config USB_OHCI_HCD_PS2
+   tristate "OHCI support for the Sony PlayStation 2"
+   depends on SONY_PS2
+   default y
+   help
+Enable support for the Sony PlayStation 2 OHCI controller.
+
 config USB_CNS3XXX_OHCI
bool "Cavium CNS3XXX OHCI Module (DEPRECATED)"
depends on ARCH_CNS3XXX
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_USB_OHCI_HCD_S3C2410)+= ohci-s3c2410.o
 obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
 obj-$(CONFIG_USB_OHCI_HCD_PXA27X)  += ohci-pxa27x.o
 obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
+obj-$(CONFIG_USB_OHCI_HCD_PS2) += ohci-ps2.o
 
 obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
new file mode 100755
--- /dev/null
+++ b/drivers/usb/host/ohci-ps2.c
@@ -0,0 +1,338 @@
+/*
+ * USB OHCI HCD (Host Controller Driver) for the PlayStation 2.
+ *
+ * Copyright (C) 2017 Jürgen Urban
+ * Copyright (C) 2017 Fredrik Noring
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "ohci.h"
+
+/* Enable USB OHCI hardware. */
+#define DPCR2_ENA_USB 0x0800
+
+/* Enable PS2DEV (required for PATA and USB). */
+#define DPCR2_ENA_PS2DEV 0x0080
+
+#define DRIVER_DESC "OHCI PS2 driver"
+#define DRV_NAME "ohci-ps2"
+
+/* Size allocated from IOP heap (maximum size of DMA memory). */
+#define DMA_BUFFER_SIZE (256 * 1024)
+
+/* Get driver private data. */
+#define hcd_to_priv(hcd) (struct ps2_hcd *)(hcd_to_ohci(hcd)->priv)
+
+struct ps2_hcd {
+   void __iomem *dpcr2;/* FIXME: Is a lock required for DPCR2? */
+   dma_addr_t iop_dma_addr;
+   bool wakeup;/* Saved wake-up state for resume. */
+};
+
+static struct hc_driver __read_mostly ohci_ps2_hc_driver;
+
+static void ohci_ps2_enable(struct usb_hcd *hcd)
+{
+   struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+
+   BUG_ON(!ohci->regs);
+
+   /* This is needed to get USB working on PS2. */
+   ohci_writel(ohci, 1, &ohci->regs->roothub.portstatus[11]);
+}
+
+static void ohci_ps2_disable(struct usb_hcd *hcd)
+{
+   struct ohci_hcd *ohci = hcd_to_ohci(hcd);
+
+   WARN_ON(!ohci->regs);
+
+   if (ohci->regs)
+   ohci_writel(ohci, 0, &ohci->regs-&

Re: [RFC v2] Add support for Sony PS2 OHCI

2017-11-24 Thread Fredrik Noring
atic int __init ps2_board_setup(void)
if (load_module_firmware("ps2/dev9_dma.irx", 0) < 0)
pr_err("loading ps2/dev9_dma.irx failed\n");
 
+   platform_add_devices(ps2_platform_devices, 
ARRAY_SIZE(ps2_platform_devices));
+
return 0;
 }
 arch_initcall(ps2_board_setup);
diff --git a/drivers/usb/host/Kconfig b/drivers/usb/host/Kconfig
index fa5692dec832..0f74d3420066 100644
--- a/drivers/usb/host/Kconfig
+++ b/drivers/usb/host/Kconfig
@@ -571,6 +571,13 @@ config USB_OHCI_EXYNOS
help
 Enable support for the Samsung Exynos SOC's on-chip OHCI controller.
 
+config USB_OHCI_HCD_PS2
+   tristate "OHCI support for the Sony PlayStation 2"
+   depends on SONY_PS2
+   default y
+   help
+Enable support for the Sony PlayStation 2 OHCI controller.
+
 config USB_CNS3XXX_OHCI
bool "Cavium CNS3XXX OHCI Module (DEPRECATED)"
depends on ARCH_CNS3XXX
diff --git a/drivers/usb/host/Makefile b/drivers/usb/host/Makefile
index 4ab2689c8952..2c6546fb2de6 100644
--- a/drivers/usb/host/Makefile
+++ b/drivers/usb/host/Makefile
@@ -60,6 +60,7 @@ obj-$(CONFIG_USB_OHCI_HCD_S3C2410)+= ohci-s3c2410.o
 obj-$(CONFIG_USB_OHCI_HCD_LPC32XX) += ohci-nxp.o
 obj-$(CONFIG_USB_OHCI_HCD_PXA27X)  += ohci-pxa27x.o
 obj-$(CONFIG_USB_OHCI_HCD_DAVINCI) += ohci-da8xx.o
+obj-$(CONFIG_USB_OHCI_HCD_PS2) += ohci-ps2.o
 
 obj-$(CONFIG_USB_UHCI_HCD) += uhci-hcd.o
 obj-$(CONFIG_USB_FHCI_HCD) += fhci.o
diff --git a/drivers/usb/host/ohci-hcd.c b/drivers/usb/host/ohci-hcd.c
index 44924824fa41..485ded7469ac 100644
--- a/drivers/usb/host/ohci-hcd.c
+++ b/drivers/usb/host/ohci-hcd.c
@@ -590,6 +590,11 @@ static int ohci_run (struct ohci_hcd *ohci)
}
udelay (1);
}
+#ifdef CONFIG_SONY_PS2
+   /* Enable USB. Leave PS2DEV enabled. */
+   outl(inl(0x1F801570) | 0x0880, 0x1F801570);
+   outl(1, 0x1F801680);
+#endif
 
/* now we're in the SUSPEND state ... must go OPERATIONAL
 * within 2msec else HC enters RESUME
@@ -875,6 +880,10 @@ static irqreturn_t ohci_irq (struct usb_hcd *hcd)
/* We only care about interrupts that are enabled */
ints &= ohci_readl(ohci, ®s->intrenable);
 
+#ifdef CONFIG_SONY_PS2
+   ohci_writel(ohci, OHCI_INTR_MIE, ®s->intrdisable);
+#endif
+
    /* interrupt for some other device? */
if (ints == 0 || unlikely(ohci->rh_state == OHCI_RH_HALTED))
return IRQ_NOTMINE;
diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
new file mode 100755
index ..c160a1649500
--- /dev/null
+++ b/drivers/usb/host/ohci-ps2.c
@@ -0,0 +1,183 @@
+/*
+ * USB OHCI HCD (Host Controller Driver) for the PlayStation 2.
+ *
+ * Copyright (C) 2010 Jürgen Urban
+ * Copyright (C) 2017 Fredrik Noring
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+
+#include "ohci.h"
+
+#define DRIVER_DESC "OHCI PS2 driver"
+
+#define DMA_BUFFER_SIZE (256 * 1024) /* Size allocated from IOP heap. */
+
+static dma_addr_t iop_dma_addr = 0;
+
+static const char hcd_name[] = "ohci-ps2";
+static struct hc_driver __read_mostly ohci_ps2_hc_driver;
+
+static int ohci_ps2_reset(struct usb_hcd *hcd)
+{
+   const int ret = ohci_setup(hcd);
+
+   /*
+* Native scatter-gather support needs to be disabled since
+* HCD_LOCAL_MEM and dma_declare_coherent_memory() are used
+* to enforce the host controller's local memory utilization,
+* otherwise hcd_alloc_coherent() in map_urb_for_dma() is
+* called with urb->transfer_buffer == NULL, that triggers a
+* NULL pointer dereference.
+*/
+   hcd->self.sg_tablesize = 0;
+
+   return ret;
+}
+
+static int iopheap_alloc_coherent(struct device *dev, size_t size, int flags)
+{
+   if (iop_dma_addr != 0)
+   return 0;
+
+   iop_dma_addr = ps2sif_allociopheap(size);
+   if (iop_dma_addr == 0) {
+   dev_err(dev, "ps2sif_allociopheap failed\n");
+   return -ENOMEM;
+   }
+
+   if (dma_declare_coherent_memory(dev, ps2sif_bustophys(iop_dma_addr),
+   iop_dma_addr, size, flags)) {
+   dev_err(dev, "dma_declare_coherent_memory failed\n");
+   ps2sif_freeiopheap(iop_dma_addr);
+   return -ENOMEM;
+   }
+
+   return 0;
+}
+
+static void iopheap_free_coherent(struct device *dev)
+{
+   if (iop_dma_addr == 0)
+   return;
+
+   dma_release_declared_memory(dev);
+   ps2sif_freeiopheap(iop_dma_addr);
+   iop_dma_addr = 0;
+}
+
+static int ohci_hcd_ps2

[RFC] Add support for Sony PS2 OHCI

2017-11-18 Thread Fredrik Noring
Hello USB maintainers,

I'm porting kernel support for the Sony PlayStation 2 from v2.6.35 to v4.14,
and a part of this effort is to port the PS2 OHCI driver. USB keyboards seem
to work quite well with v4.14, but USB mass storage devices cause kernel
crashes.

Any help in finding the cause of this crash, as well as comments on how to
improve this provisional driver are very much welcome. The crash trace is:

[ cut here ]
WARNING: CPU: 0 PID: 34 at drivers/usb/core/hcd.c:1390 
hcd_alloc_coherent+0x4c/0xc8
Modules linked in:
CPU: 0 PID: 34 Comm: usb-storage Not tainted 4.14.0+ #718
Stack :  80402608 0003 000b 805b3b98 805611c4 0094 
80062b8c
0009 056e 81e9fc00 805b 0094 10018c00 81f51b10 
0001
  805b 0001 000a 0003 0001 
3a555043
80436790 000f 81f519c8 81f51990   802e544c 
80517464
0009 056e 81e9fc00 805b 0003 ee7f  
805b
...
Call Trace:
[<80021104>] show_stack+0x74/0x104
[<80036310>] __warn+0x114/0x11c
[<800363ac>] warn_slowpath_null+0x1c/0x30
[<802e544c>] hcd_alloc_coherent+0x4c/0xc8
[<802e6160>] usb_hcd_map_urb_for_dma+0x524/0x580
[<802e729c>] usb_hcd_submit_urb+0x7d8/0x7e0
[<802e95d8>] usb_sg_wait+0x14c/0x1a0
[<802fb0f0>] usb_stor_bulk_transfer_sglist.part.1+0xac/0x124
[<802fb4b4>] usb_stor_bulk_srb+0x40/0x60
[<802fb8a8>] usb_stor_Bulk_transport+0x160/0x37c
[<802fbcec>] usb_stor_invoke_transport+0x3c/0x500
[<802fd220>] usb_stor_control_thread+0x260/0x2a0
[<8004ef64>] kthread+0x138/0x140
[<8001b578>] ret_from_kernel_thread+0x14/0x1c
---[ end trace 2be087478be6627e ]---

This particular WARN_ON_ONCE is explained here:

commit 4307a28eb0128417d9a2b9d858d2bce70ee5b383
Author: Andrea Righi 
Date:   Mon Jun 28 16:56:45 2010 +0200

USB: EHCI: fix NULL pointer dererence in HCDs that use HCD_LOCAL_MEM

If we use the HCD_LOCAL_MEM flag and dma_declare_coherent_memory() to
enforce the host controller's local memory utilization we also need to
disable native scatter-gather support, otherwise hcd_alloc_coherent() in
map_urb_for_dma() is called with urb->transfer_buffer == NULL, that
triggers a NULL pointer dereference.

We can also consider to add a WARN_ON() and return an error code to
better catch this problem in the future.

At the moment no driver seems to hit this bug, so I should
consider this a low-priority fix.

Signed-off-by: Andrea Righi 
Acked-by: Alan Stern 
Signed-off-by: Greg Kroah-Hartman 

I've tried to disable scatter-gather, as suggested, by:

diff --git a/drivers/usb/host/ohci-ps2.c b/drivers/usb/host/ohci-ps2.c
index 066d6a9c7ccb..eaa099aa0740 100755
--- a/drivers/usb/host/ohci-ps2.c
+++ b/drivers/usb/host/ohci-ps2.c
@@ -25,6 +25,9 @@ static int ohci_ps2_start(struct usb_hcd *hcd)
 
ohci_hcd_init(ohci);
ohci_init(ohci);
+
+   hcd->self.sg_tablesize = 0;
+
ohci_run(ohci);
hcd->state = HC_STATE_RUNNING;
return 0;

With the patch above, the kernel successfully detects USB mass storage
devices. However, it also causes another crash:

[ cut here ]
WARNING: CPU: 0 PID: 15 at ./include/linux/dma-mapping.h:530 
hcd_buffer_free+0x130/0x200
Modules linked in:
CPU: 0 PID: 15 Comm: kworker/0:1 Not tainted 4.14.0+ #719
Workqueue: usb_hub_wq hub_event
Stack : 81f3b5b8 80402608 0003 000b 00f6 805611c4 81ccc280 
804f398c
802ee5e0 804fd520 0009 0212 0001 10058400 81c09a38 
0001
   0003 000a 0002 0001 

0001 3368 81c09878 81c09840   802ee5e0 
804fd520
0009 0212 0001 fff3 0002   
805b
...
Call Trace:
[<80021104>] show_stack+0x74/0x104
[<80036310>] __warn+0x114/0x11c
[<800363ac>] warn_slowpath_null+0x1c/0x30
[<802ee5e0>] hcd_buffer_free+0x130/0x200
[<802e56ec>] usb_hcd_unmap_urb_for_dma+0x160/0x16c
[<802e576c>] __usb_hcd_giveback_urb+0x54/0xf0
[<802f5d64>] finish_urb+0x98/0x138
[<802f716c>] ohci_work+0x14c/0x494
[<802f9878>] ohci_irq+0x13c/0x2e4
[<802e4e0c>] usb_hcd_irq+0x2c/0x44
[<8006366c>] __handle_irq_event_percpu+0x98/0x158
[<8006374c>] handle_irq_event_percpu+0x20/0x64
[<800637cc>] handle_irq_event+0x3c/0x70
[<80067024>] handle_level_irq+0xe4/0x120
[<80062d70>] generic_handle_irq+0x28/0x38
[<80409b58>] do_IRQ+0x18/0x24
---[ end trace fa1f0201799de649 ]---

Please find the (provisio