[illumos-Developer] vmxnet3s driver failing on debug builds due to dma_attr_sgllen == -1 (Bug #422)

[Yuri Pankov]

On Sat, Feb 05, 2011 at 09:35:55PM -0800, Garrett D'Amore wrote:
> I've finally looked at this.  I agree, its a bug for DEBUG kernels.
> Lets file a bug in illumos, and we'll fix it.
> 
> 	- Garrett
> 
> On Sat, 2011-02-05 at 23:19 -0600, Mike La Spina wrote:
> > I would think that the debug call code is not appropriate. There is
> > nothing wrong with the -1 value and it should not return a bad attribute
> > when it's valid.
> > 

That's cool and all. Could you comment on the issue please? Why the
check was there? Aren't we doing something bad removing the check (may
be we should make it more clearer)?

> > -----Original Message-----
> > From: Yuri Pankov [mailto:yuri.pankov at gmail.com] 
> > Sent: Saturday, February 05, 2011 10:54 AM
> > To: Garrett D'Amore
> > Cc: developer at lists.illumos.org
> > Subject: Re: [illumos-Developer] vmxnet3s driver failing on debug builds
> > due to dma_attr_sgllen == -1 (Bug #422)
> > 
> > On Sat, Feb 05, 2011 at 07:34:56AM -0800, Garrett D'Amore wrote:
> > > On Sat, 2011-02-05 at 10:29 +0300, Yuri Pankov wrote:
> > > > Hi,
> > > > 
> > > > While looking into the issue, I've found the following piece of code
> > in
> > > > usr/src/uts/i86pc/io/rootnex.c, called only in debug builds:
> > > > 
> > > > /*
> > > >  * rootnex_valid_alloc_parms()
> > > >  *    Called in ddi_dma_alloc_handle path to validate its
> > parameters.
> > > >  */
> > > > static int
> > > > rootnex_valid_alloc_parms(ddi_dma_attr_t *attr, uint_t
> > maxsegmentsize)
> > > > ...
> > > >         if ((attr->dma_attr_seg & MMU_PAGEOFFSET) != MMU_PAGEOFFSET
> > ||
> > > >             MMU_PAGESIZE & (attr->dma_attr_granular - 1) ||
> > > > ==>         attr->dma_attr_sgllen <= 0) {          <===
> > > >                 return (DDI_DMA_BADATTR);
> > > >         }
> > > > ...
> > > > 
> > > > vmxnet3s driver uses dma_attr_sgllen == -1, which is pretty ok
> > according
> > > > to ddi_dma_attr(9S):
> > > > ...
> > > >      < 0    Device DMA engine is not constrained by the size, for
> > > >             example, withDMA chaining.
> > > > ...
> > > > 
> > > > Removing the check allows the driver to successfully start (and
> > > > function, I haven't tested the 10Gbps speed due to the lack of
> > hardware,
> > > > but currently finishing 50GB transfer at 1Gbps speed). The check is
> > > > there from the OpenSolaris launch, so the my real question is - what
> > of
> > > > the following sounds correct:
> > > > - the check is bogus and should be removed (or changed to == 0,
> > which is
> > > >   reserved per manpage)
> > > > or
> > > > - documentation (ddi_dma_attr(9S), at least) should be updated and
> > the
> > > >   driver should be "fixed".
> > > > 
> > > 
> > > I haven't looked yet, but lots of drivers have sgllen == -1.  That
> > means
> > > your driver can accept arbitrary chaining via linked lists.  I expect
> > > there is something else going on here, and I'll look at more closely
> > as
> > > soon as I have time.
> > > 
> > > 	- Garrett
> > 
> > Thanks Garrett.
> > 
> > I just wasn't able to find any driver in tree using sgllen = -1 (that
> > was just some grepping though) and I don't really see how allocation
> > could succeed given the codepath. Could you please provide some examples
> > of drivers using sgllen = -1? Sorry if I'm missing something obvious
> > here, have much to learn yet..
> > 
> > 
> > TIA,
> > Yuri
> > 
> > _______________________________________________
> > Developer mailing list
> > Developer at lists.illumos.org
> > http://lists.illumos.org/m/listinfo/developer
> 
>