VIMAGE hang, USB panic

[Marius Strobl]

On Mon, Jul 26, 2010 at 08:59:20AM +0200, Hans Petter Selasky wrote:
> On Sunday 25 July 2010 16:32:52 Nathaniel W Filardo wrote:
> > Speaking of USB hotplug, attaching a micro 4-port hub, I get this (both
> > with and without VIMAGE):
> > 
> > ugen0.2: <vendor 0x05e3> at usbus0
> > uhub1: <vendor 0x05e3 USB2.0 Hub, class 9/0, rev 2.00/9.01, addr 2> on
> > usbus0
> > panic: iommu_dvmamap_create: bogus preallocation size , nsegments = 2,
> > maxpre = 2, maxsize = 1
> > cpuid = 0
> > KDB: stack backtrace:
> 
> Hi,
> 
> Looks like the following check fails. The allocation size USB requests is 1-
> byte. For the check to not fail, we need to request more bytes then the max-
> segments parameter specified.

Or decrease the nsegments parameter. I think that the sanity check is
right and the parameters supplied are bogus as 1 byte cannot be split
across 2 segments.

> 
>         maxpre = imin(dt->dt_nsegments, IOMMU_MAX_PRE_SEG);
>         presz = dt->dt_maxsize / maxpre;
>         KASSERT(presz != 0, ("%s: bogus preallocation size , nsegments = %d, "
>             "maxpre = %d, maxsize = %lu", __func__, dt->dt_nsegments, maxpre,
>             dt->dt_maxsize));
> 
> Does not look like a problem in the USB stack.

I think it is as the maxsize and nsegments parameters supplied in this
case make no sense. Moreover, maxsegsz should be <= maxsize, thus I'd
like to commit the following change:

Index: usb_busdma.c
===================================================================
--- usb_busdma.c	(revision 212478)
+++ usb_busdma.c	(working copy)
@@ -366,9 +366,9 @@ usb_dma_tag_create(struct usb_dma_tag *udt,
 	     /* filter    */ NULL,
 	     /* filterarg */ NULL,
 	     /* maxsize   */ size,
-	     /* nsegments */ (align == 1) ?
+	     /* nsegments */ (align == 1 && size > 1) ?
 	    (2 + (size / USB_PAGE_SIZE)) : 1,
-	     /* maxsegsz  */ (align == 1) ?
+	     /* maxsegsz  */ (align == 1 && size > USB_PAGE_SIZE) ?
 	    USB_PAGE_SIZE : size,
 	     /* flags     */ BUS_DMA_KEEP_PG_OFFSET,
 	     /* lockfn    */ &usb_dma_lock_cb,

Why is this using one more segment than splitting size in USB_PAGE_SIZE
sized segments btw? If this isn't actually needed then replacing the 2
with a 1 would be the more preferable fix IMO.

However, specifying such small allocation sizes together with
BUS_SPACE_UNRESTRICTED as nsegments, which unlike what usb(4)
currently does the I'd would consider as valid combinations,
will also trigger this assertion, so I'll remove it.

Marius