On 2002.04.16 04:21 Jens Owen wrote:
> Jose,
> 
> Was your concern addressed in today's chat?
> 

Not really... See below.

> Jose Fonseca wrote:
> >
> > I have been looking into the DRM's DMA/CCE/CP initialization
> subroutines
> > (those named *_do_init{dma,cce,cp} ) of the current DRI drivers, and I
> > still don't know how they protect themselves from making faults due to
> > bad initialization parameters. Since I this is a little complicated to
> > put over IRC I'm sending this email to prepare you for tonight...
> >
> > The first question is why the dev->dev_private initializition is
> delayed
> > until the last moment of exiting these subroutines. Taking the Radeon
> > code as example, why is radeon_do_init_cp (radeon_cp.c:668) full of
> > statements like
> >
> >                 dev->dev_private = (void *)dev_priv;
> >
> > instead of a single on right after dev_priv allocation (as done
> before)?
> > Is this subroutine reentrant? Even if it is, how does this solve the
> > problem?
> >

I got no answers to this first question.

> > The other thing is in r128_do_cleanup_cce. This subroutine is called by
> > r128_do_init_cce in case of failure as in
> >
> >         DRM_FIND_MAP( dev_priv->buffers, init->buffers_offset );
> >         if(!dev_priv->buffers) {
> >                 DRM_ERROR("could not find dma buffer region!\n");
> >                 dev->dev_private = (void *)dev_priv;
> >                 r128_do_cleanup_cce( dev );
> >                 return -EINVAL;
> >         }
> >
> > but if indeed dev_priv->buffers is NULL, won't the following code in
> > 128_do_cleanup_cce generate a segfault?
> >
> >                 if ( !dev_priv->is_pci ) {
> >                         DRM_IOREMAPFREE( dev_priv->cce_ring );
> >                         DRM_IOREMAPFREE( dev_priv->ring_rptr );
> >                         DRM_IOREMAPFREE( dev_priv->buffers );
> >                 } else {
> >
> > since as said in a previous email of mine DRM_IOREMAPFREE doesn't check
> > if its argument is NULL, on the contrary it references it before
> passing
> > it to ioremapfree:
> >
> > #define DRM_IOREMAPFREE(map)
> \
> >         do {
> \
> >                 if ( (map)->handle && (map)->size )
> \
> >                         DRM(ioremapfree)( (map)->handle, (map)->size
> );\
> >         } while (0)
> >
> > So isn't there a bug here as well or I'm missing something?
> >

Regarding my sencond question, although Michel D�nzer wasn't the author of 
this r128 code he said he seemed to agree with me, but since that code 
construct also happens in several other drivers -- in fact I think it 
happens in every single one --, I would like to get a global consensus 
that we have a problem and what is the best way to solve it, before start 
to make changes everywhere.


> > Just in case you may be thinking that this can be fairly uncommon this
> > situation occurs when the init structure changes. So everytime there is
> > a versioning problem in the DRM, libdrm.a or DDX which involves a
> change
> > on the init structure this will most likely to occur and, at least on
> my
> > systems, it locks mach64.o on the kernel, and only a reset can allow
> > normal operation again.
> >
> > Jose Fonseca
> >
> 
> --                             /\
>          Jens Owen            /  \/\ _
>   [EMAIL PROTECTED]  /    \ \ \   Steamboat Springs, Colorado
> 


Jos� Fonseca

_______________________________________________
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel

Reply via email to