Date: Mon, 13 Sep 2010 13:58:23 +0200 From: Hans Petter Selasky <hselasky@c2i.net> To: Marius Strobl <marius@alchemy.franken.de> Cc: Nathaniel W Filardo <nwf@cs.jhu.edu>, freebsd-sparc64@freebsd.org Subject: Re: VIMAGE hang, USB panic Message-ID: <201009131358.24323.hselasky@c2i.net> In-Reply-To: <20100912182214.GA90233@alchemy.franken.de> References: <20100725143252.GV21929@gradx.cs.jhu.edu> <201007260859.20501.hselasky@c2i.net> <20100912182214.GA90233@alchemy.franken.de>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sunday 12 September 2010 20:22:14 Marius Strobl wrote: > 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: Hi, Yes, the values supplied are not too tight. The patch you've made to usb_busdma.c seems OK to me. > > 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. We are using 2 because a virtual memory load operation of less than USB_PAGE_SIZE, can be split accross two pages at maximum. > > 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. It is usually not allocation that are of this size, but rather DMA load mappings. --HPS
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201009131358.24323.hselasky>