Date: Sun, 12 Sep 2010 20:22:14 +0200 From: Marius Strobl <marius@alchemy.franken.de> To: Hans Petter Selasky <hselasky@c2i.net> Cc: Nathaniel W Filardo <nwf@cs.jhu.edu>, freebsd-sparc64@freebsd.org Subject: Re: VIMAGE hang, USB panic Message-ID: <20100912182214.GA90233@alchemy.franken.de> In-Reply-To: <201007260859.20501.hselasky@c2i.net> References: <20100725143252.GV21929@gradx.cs.jhu.edu> <201007260859.20501.hselasky@c2i.net>
index | next in thread | previous in thread | raw e-mail
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
home |
help
Want to link to this message? Use this
URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100912182214.GA90233>