Re: [Dri-devel] DRM OS Independence and Mach64
On Tue, Feb 18, 2003 at 09:58:41PM -0500, Leif Delgass wrote: > On Wed, 19 Feb 2003, [iso-8859-15] José Fonseca wrote: > > It's now official: I'm coding on this. I'm adding the _ioctl suffix to > > most IOCTLs (e.g., agp_alloc -> agp_alloc_ioctl) to leave to the > > agp_alloc for internal DRM usage, and writing a set of pci_* to wrap the > > pci_*_consistent and pci_pool_* API in the Linux. > > > > Nothing of this will break binary compatability and will allow one to > > [optionally] setup all main DMA buffers from within DRM_IOCTL_DMA IOCTL > > instead of the X server. Of course that I would like to pursue this idea > > in Mach64 driver. > > I'll be interested to see what you come up with. How much of the setup in > atidri.c were you thinking of moving to the kernel? We'd still need to > allow for XF86Config options dealing with the AGP aperture and buffer > region size(s), as well as passing a handles back to the Xserver for the > AGP aperture (to setup the AGP registers) and the AGP texture map > (although maybe the Mesa client could just get it with a get_param ioctl). > If the Xserver does it, it needs to leave space for the private buffers in > AGP when calculating the region sizes/offsets. ATIDRIAddBuffers and ATIDRIMapBuffers will cease to exist as the drmAddBufs and drmMapBufs should be considered deprecated. This will be done in the DRM but I'll add a new APIs to allow do this from X if ever wanted. ATIDRIAgpInit will just process XF86Config options. The AGP setup and the AGP register initialization can both be done in the DRM. Everything related with the onboard will remain to be done on XFree86, at least for the time being. The onboard memory managament will soon have to be rethinked, either for Ian Romanick's texmem-2 or for XFree86 RandR frambuffer resizing. > > > BTW, is it OK to enable snapshots from the 0-0-6 branch instead? > > I think that would be fine, but I think that might mean that the extras > package will be required too. I was forgetting about that. It's better not to then... > BTW, what's the status of the trunk snaphots? Are we waiting for 4.3.0 > to start those again? Yep, 4.3.0 to be released and to be adopted by the main distros. Many distros already have made 4.2.99.* available so it shouldn't take long. José Fonseca __ Do You Yahoo!? Everything you'll ever need on one web page from News and Sport to Email and Music Charts http://uk.my.yahoo.com --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Wed, 19 Feb 2003, [iso-8859-15] José Fonseca wrote: > On Tue, Feb 18, 2003 at 06:30:40PM -0500, Leif Delgass wrote: > > I also made some other changes to the copy/verify: > > > > I added a check to the ioctl handler (mach64_dma_vertex) to check that the > > vertex buffer has an integral number of dwords (in addition to the check > > against MACH64_BUFFER_SIZE). I also changed copy_and_verify_from_user to > > return an error code instead of the number of bytes copied. I don't see > > any reason to dispatch a buffer unless all checks succeed (in fact it can > > potentially cause lockups if we submit a partial buffer), so it's either > > all or nothing. If it succeeds then we just copy buf->used to the private > > buffer struct. This also allows us to return a more meaningful error > > code, e.g. EFAULT for failed verify_area/copy_from_user, EINVAL for bad > > command counts or buffer size, and EACCES for disallowed register commands > > in the buffer. It also lets us get rid of the 'copied' local var and a > > couple of adds inside the loop. > > At that time I coded that way as an aid to debug that code. Now I don't > see any problem in letting that go. > > > A couple of other minor tweaks I made > > were copying the byte length parameter to a local var and declaring the > > function as inline. It also now will generate an error if there is a > > register command without at least one data dword following it (in addition > > to checking for a command count that overflows buf->used). That check is > > now the new loop condition (n > 1 instead of n (!=0) ), so it doesn't add > > anything inside the loop. There is one extra conditional in the loop now > > to check the return of copy_from_user. All of this may or may not have > > much of a performance effect, but it is a pretty critical code path, I > > think. > > My impression is that the effect of the copy/verify is noticeable in > slower machines but neglectable in faster. But I guess that the hit is > probably more related to the processor cache than its actual speed. At some point, I'm planning on trying out oprofile with the new branch. Last time I looked, a lot of time seemed to be spent in freelist_get waiting for free buffers, but I think that was also before copy/verify. > > > > 3. We still need to work out the wrapper/alternative to > > > > pci_alloc_consistent() and friends. > > > > > > I'm still reading Ian texmem-2 proposal and looking to the source code > > > to get a hold of this. > > > > It's now official: I'm coding on this. I'm adding the _ioctl suffix to > most IOCTLs (e.g., agp_alloc -> agp_alloc_ioctl) to leave to the > agp_alloc for internal DRM usage, and writing a set of pci_* to wrap the > pci_*_consistent and pci_pool_* API in the Linux. > > Nothing of this will break binary compatability and will allow one to > [optionally] setup all main DMA buffers from within DRM_IOCTL_DMA IOCTL > instead of the X server. Of course that I would like to pursue this idea > in Mach64 driver. I'll be interested to see what you come up with. How much of the setup in atidri.c were you thinking of moving to the kernel? We'd still need to allow for XF86Config options dealing with the AGP aperture and buffer region size(s), as well as passing a handles back to the Xserver for the AGP aperture (to setup the AGP registers) and the AGP texture map (although maybe the Mesa client could just get it with a get_param ioctl). If the Xserver does it, it needs to leave space for the private buffers in AGP when calculating the region sizes/offsets. > > Something I have in my old tree that I haven't merged to the new branch > > yet is setting up the ring in AGP when available. That will get rid of > > the use of pci_alloc_consistent for AGP cards. However we still can't do > > private buffers in AGP without multiple buffer lists. However, it might > > be enough to allow porting/testing on *BSD to continue with the PCI mode > > setup in mach64_dma.c disabled (with the current temporary code that > > uses client buffers instead of secure buffers). > > BTW, is it OK to enable snapshots from the 0-0-6 branch instead? I think that would be fine, but I think that might mean that the extras package will be required too. BTW, what's the status of the trunk snaphots? Are we waiting for 4.3.0 to start those again? -- Leif Delgass http://www.retinalburn.net --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Tue, 18 Feb 2003, Linus Torvalds wrote:
>
> On 19 Feb 2003, Alan Cox wrote:
> >
> > copy_from_user sorts out the whole thing itself. In general
> > __copy_from_user and verify_area isnt a win, except if you do lots of
> > small copies to/from the same area, which is often a bad idea anyway.
>
> It _can_ be a good idea, though.
>
> In particular, it's a _really_ good idea if the loop is basically a
> "glorified memcpy", at which point it can be very much worthwhile to move
> the limit checking outside the loop.
>
> An example of this is doing things like "copy-and-checksum", or, in the
> case of DRM "copy-and-check-validity", where the code should sanely look
> something like
>
>
> u32 *addr; /* Command list */
>
> if (!access_ok(VERIFY_READ, addr, size))
> return -EFAULT;
>
> while (size > 0) {
> unsigned int cmd;
>
> if (__get_user(cmd, addr))
> goto efault;
> addr++;
> switch (cmd) {
> ... verify it ...
> }
>
> *dst++ = cmd;
> }
>
> where the difference between a "get_user()" and a "__get_user()" can be
> quite noticeable (the unchecked version is usually a single instruction in
> size and has no nasty branch behaviour or anything like that, while the
> checking version usually has at least one conditional branch and uses up
> one or two registers etc).
>
> Of course, this is really only worth doing for tight loops that really
> matter. If the code in the loop is crap, the optimization just isn't worth
> the complexity.
>
> (And the extreme case of this is when the code is _so_ performance
> critical that you end up writing it all in assembly language. The example
> of this is the TCP copy-and-checksum stuff. The above example is somewhere
> between the "normal usage" and the "TCP copy-and-checksum" extremes).
>
> Linus
The code is similar to this, but with a couple of differences. We have to
split apart 'cmd' into two local vars (swapping bytes for big-endian) and
verify them (register address and count of data dwords following). If the
verify passes, we copy the command as in the last line of the loop above,
but then we need to __copy_from_user up to 7 dwords of data into the
destination without reading it. So it's a loop of alternating 1 dword
read/verify/write and 1-7 dwords straight copy.
--
Leif Delgass
http://www.retinalburn.net
---
This SF.net email is sponsored by: SlickEdit Inc. Develop an edge.
The most comprehensive and flexible code editor you can use.
Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial.
www.slickedit.com/sourceforge
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Tue, Feb 18, 2003 at 06:30:40PM -0500, Leif Delgass wrote: > I also made some other changes to the copy/verify: > > I added a check to the ioctl handler (mach64_dma_vertex) to check that the > vertex buffer has an integral number of dwords (in addition to the check > against MACH64_BUFFER_SIZE). I also changed copy_and_verify_from_user to > return an error code instead of the number of bytes copied. I don't see > any reason to dispatch a buffer unless all checks succeed (in fact it can > potentially cause lockups if we submit a partial buffer), so it's either > all or nothing. If it succeeds then we just copy buf->used to the private > buffer struct. This also allows us to return a more meaningful error > code, e.g. EFAULT for failed verify_area/copy_from_user, EINVAL for bad > command counts or buffer size, and EACCES for disallowed register commands > in the buffer. It also lets us get rid of the 'copied' local var and a > couple of adds inside the loop. At that time I coded that way as an aid to debug that code. Now I don't see any problem in letting that go. > A couple of other minor tweaks I made > were copying the byte length parameter to a local var and declaring the > function as inline. It also now will generate an error if there is a > register command without at least one data dword following it (in addition > to checking for a command count that overflows buf->used). That check is > now the new loop condition (n > 1 instead of n (!=0) ), so it doesn't add > anything inside the loop. There is one extra conditional in the loop now > to check the return of copy_from_user. All of this may or may not have > much of a performance effect, but it is a pretty critical code path, I > think. My impression is that the effect of the copy/verify is noticeable in slower machines but neglectable in faster. But I guess that the hit is probably more related to the processor cache than its actual speed. > > > 3. We still need to work out the wrapper/alternative to > > > pci_alloc_consistent() and friends. > > > > I'm still reading Ian texmem-2 proposal and looking to the source code > > to get a hold of this. > It's now official: I'm coding on this. I'm adding the _ioctl suffix to most IOCTLs (e.g., agp_alloc -> agp_alloc_ioctl) to leave to the agp_alloc for internal DRM usage, and writing a set of pci_* to wrap the pci_*_consistent and pci_pool_* API in the Linux. Nothing of this will break binary compatability and will allow one to [optionally] setup all main DMA buffers from within DRM_IOCTL_DMA IOCTL instead of the X server. Of course that I would like to pursue this idea in Mach64 driver. > Something I have in my old tree that I haven't merged to the new branch > yet is setting up the ring in AGP when available. That will get rid of > the use of pci_alloc_consistent for AGP cards. However we still can't do > private buffers in AGP without multiple buffer lists. However, it might > be enough to allow porting/testing on *BSD to continue with the PCI mode > setup in mach64_dma.c disabled (with the current temporary code that > uses client buffers instead of secure buffers). BTW, is it OK to enable snapshots from the 0-0-6 branch instead? José Fonseca __ Do You Yahoo!? Everything you'll ever need on one web page from News and Sport to Email and Music Charts http://uk.my.yahoo.com --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Wed, 2003-02-19 at 01:48, José Fonseca wrote: > I think our case is one of the exceptions: we are copying from the user > and verifying the data at the same time (to avoid the user sending > malicious commands to the card's DMA engine which could potentialy > compromise the system integrity and security). The alternative would be > copy the whole thing and the verify but it was agreed that this would > make better use of the cache and so on... Small here is bytes, caches are 64byte lines nowdays, sometimes up to 256 or so to the L2 cache. Time it and see 8) --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Wed, Feb 19, 2003 at 01:50:49AM +, Alan Cox wrote: > > verify_area performs any checks needed for it to be safe to use the > _copy forms of copy_*_user. What that does varies. The normal Linux > approach is > > verify_area does access_ok which checks the address is a kernel view > of a user space address. _copy* functions use the MMU to fault any > illegal access beyond that. > > On some platforms you cant use the mmu so verify_area does more work, > on others its using stuff like space registers and verify_area is a > nop. Thanks for the clarification. > copy_from_user sorts out the whole thing itself. In general > __copy_from_user and verify_area isnt a win, except if you do lots of > small copies to/from the same area, which is often a bad idea anyway. I think our case is one of the exceptions: we are copying from the user and verifying the data at the same time (to avoid the user sending malicious commands to the card's DMA engine which could potentialy compromise the system integrity and security). The alternative would be copy the whole thing and the verify but it was agreed that this would make better use of the cache and so on... José Fonseca __ Do You Yahoo!? Everything you'll ever need on one web page from News and Sport to Email and Music Charts http://uk.my.yahoo.com --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On 19 Feb 2003, Alan Cox wrote:
>
> copy_from_user sorts out the whole thing itself. In general __copy_from_user and
>verify_area
> isnt a win, except if you do lots of small copies to/from the same area, which is
>often
> a bad idea anyway.
It _can_ be a good idea, though.
In particular, it's a _really_ good idea if the loop is basically a
"glorified memcpy", at which point it can be very much worthwhile to move
the limit checking outside the loop.
An example of this is doing things like "copy-and-checksum", or, in the
case of DRM "copy-and-check-validity", where the code should sanely look
something like
u32 *addr; /* Command list */
if (!access_ok(VERIFY_READ, addr, size))
return -EFAULT;
while (size > 0) {
unsigned int cmd;
if (__get_user(cmd, addr))
goto efault;
addr++;
switch (cmd) {
... verify it ...
}
*dst++ = cmd;
}
where the difference between a "get_user()" and a "__get_user()" can be
quite noticeable (the unchecked version is usually a single instruction in
size and has no nasty branch behaviour or anything like that, while the
checking version usually has at least one conditional branch and uses up
one or two registers etc).
Of course, this is really only worth doing for tight loops that really
matter. If the code in the loop is crap, the optimization just isn't worth
the complexity.
(And the extreme case of this is when the code is _so_ performance
critical that you end up writing it all in assembly language. The example
of this is the TCP copy-and-checksum stuff. The above example is somewhere
between the "normal usage" and the "TCP copy-and-checksum" extremes).
Linus
---
This SF.net email is sponsored by: SlickEdit Inc. Develop an edge.
The most comprehensive and flexible code editor you can use.
Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial.
www.slickedit.com/sourceforge
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
verify_area performs any checks needed for it to be safe to use the _copy forms of copy_*_user. What that does varies. The normal Linux approach is verify_area does access_ok which checks the address is a kernel view of a user space address. _copy* functions use the MMU to fault any illegal access beyond that. On some platforms you cant use the mmu so verify_area does more work, on others its using stuff like space registers and verify_area is a nop. copy_from_user sorts out the whole thing itself. In general __copy_from_user and verify_area isnt a win, except if you do lots of small copies to/from the same area, which is often a bad idea anyway. Alan --- This SF.net email is sponsored by: SlickEdit Inc. Develop an edge. The most comprehensive and flexible code editor you can use. Code faster. C/C++, C#, Java, HTML, XML, many more. FREE 30-Day Trial. www.slickedit.com/sourceforge ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Tue, 18 Feb 2003, Leif Delgass wrote: > > My memory is failing: this might still be usefull for Xv, isn't it? > > Maybe, maybe not. DMA for XVideo seems to be of questionable value, > judging from the Rage 128 driver. Plus the fact that you have to > wait/sync when switching between GUI masters and system masters. I've > used interrupts to test page flipping with vsync. However, this is also > of questionable utility. Page flipping without vsync doesn't really buy > anything, since it has to be done synchronously (block until the vertical > counter resets). What I meant to say here is that page flipping without vsync _interrupts_ on mach64 doesn't buy you much. It's not possible to do page flipping _without_ syncing the flip to the refresh on mach64. Doing it without interrupts means polling the vertical counter: either block until the counter resets, or poll whenever you have new buffers to see if you can commit them. The latter is not much different from my interrupt method. I got interrupts working using an atomic commit flag that's set on vblank and checked when adding buffers to the ring, since I was getting lockups trying to do the commit in the bottom half. Ideally, you would commit the ring in the bottom half, so the card can work in the time between the interrupt and the arrival of the next buffer from the client. > One advantage is that it seems to smooth out some of the stutters in > rendering as well as eliminating tearing, but at the cost of reducing > the framerate if the app can't keep up with the vertical refresh. Most > apps on mach64 (other than gears) don't fall into that category. :( To clarify, here I meant that most apps fall in the category of having a framerate _slower_ than the refresh. -- Leif Delgass http://www.retinalburn.net --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Tue, 18 Feb 2003, [iso-8859-15] José Fonseca wrote:
> On Mon, Feb 17, 2003 at 08:15:02PM -0500, Leif Delgass wrote:
> > As I was doing some minor cleanups in the mach64 drm in the new branch, I
> > made some additional search and replace conversions of the mach64 DRM to
> > the os independence macros (I couldn't restrain myself ;) ). However, I
> > want to share what I've done so far and get some feedback, since there are
> > a couple of issues here:
> >
> > 1. We are currently using access_ok() to check the vertex buffers
> > submitted by the client before copying them to (what will be) private
> > kernel buffers. There was only a macro in drm_os_linux.h for
> > DRM_VERIFYAREA_READ(), so I added DRM_ACCESSOK_READ(). I don't really
> > know the details of what the difference is between verify_area() and
> > access_ok() (Jose added that code based on a suggestion from Linus, I
> > think), but I believe access_ok() is intended as a security check, which
> > is the reason for the copy. It seems that this all maps to the same thing
> > for *BSD at the moment -- i.e. the unchecked macros aren't implemented
> > differently from the checked ones, right?
>
> They seem to be equivalent, onyle the semantics of the return value
> differ. Here is the definition of verify_area in my distribution kernel
> sources (linux-2.4.20-gentoo-r1), in include/asm-i386/uaccess.h:
>
> static inline int verify_area(int type, const void * addr, unsigned long size)
> {
> return access_ok(type,addr,size) ? 0 : -EFAULT;
> }
>
> So perhaps we should converge on one.
OK, I've changed this to use DRM_VERIFYAREA_READ().
I also made some other changes to the copy/verify:
I added a check to the ioctl handler (mach64_dma_vertex) to check that the
vertex buffer has an integral number of dwords (in addition to the check
against MACH64_BUFFER_SIZE). I also changed copy_and_verify_from_user to
return an error code instead of the number of bytes copied. I don't see
any reason to dispatch a buffer unless all checks succeed (in fact it can
potentially cause lockups if we submit a partial buffer), so it's either
all or nothing. If it succeeds then we just copy buf->used to the private
buffer struct. This also allows us to return a more meaningful error
code, e.g. EFAULT for failed verify_area/copy_from_user, EINVAL for bad
command counts or buffer size, and EACCES for disallowed register commands
in the buffer. It also lets us get rid of the 'copied' local var and a
couple of adds inside the loop. A couple of other minor tweaks I made
were copying the byte length parameter to a local var and declaring the
function as inline. It also now will generate an error if there is a
register command without at least one data dword following it (in addition
to checking for a command count that overflows buf->used). That check is
now the new loop condition (n > 1 instead of n (!=0) ), so it doesn't add
anything inside the loop. There is one extra conditional in the loop now
to check the return of copy_from_user. All of this may or may not have
much of a performance effect, but it is a pretty critical code path, I
think.
> > 2. The Mach64 driver makes heavy use of the list struct and macros from
> > linux/list.h. I moved the define for list_for_each_safe() (needed for
> > older 2.4 Linux kernels) from mach64_drv.h to drmP.h, since that has
> > already been added in XFree86 CVS (I think the i8x0 drm uses it now also).
> > I also removed the include of linux/list.h from the mach64 driver, since
> > it already gets included (indirectly?) through the drm headers. However,
> > it looks like an analogue of linux/list.h might need to be added to the
> > BSD drm headers. The only wrinkle there is that it also uses
> > linux/prefetch.h.
FYI Eric, here's what we're currently using from linux/list.h:
struct list_head
INIT_LIST_HEAD()
list_entry()
list_for_each()
list_for_each_safe()
list_del()
list_add_tail()
list_empty()
> > 3. We still need to work out the wrapper/alternative to
> > pci_alloc_consistent() and friends.
>
> I'm still reading Ian texmem-2 proposal and looking to the source code
> to get a hold of this.
Something I have in my old tree that I haven't merged to the new branch
yet is setting up the ring in AGP when available. That will get rid of
the use of pci_alloc_consistent for AGP cards. However we still can't do
private buffers in AGP without multiple buffer lists. However, it might
be enough to allow porting/testing on *BSD to continue with the PCI mode
setup in mach64_dma.c disabled (with the current temporary code that
uses client buffers instead of secure buffers).
Eric, do you have hardware to test on?
> > 4. As I mentioned before, the interrupt code is not converted, but it's
> > currently unused.
>
> My memory is failing: this might still be usefull for Xv, isn't it?
Maybe, maybe not. DMA for XVideo seems to be of questionable value,
judging from the Rage 128 driver. Plus the fact that you hav
Re: [Dri-devel] DRM OS Independence and Mach64
On Mon, Feb 17, 2003 at 08:15:02PM -0500, Leif Delgass wrote:
> As I was doing some minor cleanups in the mach64 drm in the new branch, I
> made some additional search and replace conversions of the mach64 DRM to
> the os independence macros (I couldn't restrain myself ;) ). However, I
> want to share what I've done so far and get some feedback, since there are
> a couple of issues here:
>
> 1. We are currently using access_ok() to check the vertex buffers
> submitted by the client before copying them to (what will be) private
> kernel buffers. There was only a macro in drm_os_linux.h for
> DRM_VERIFYAREA_READ(), so I added DRM_ACCESSOK_READ(). I don't really
> know the details of what the difference is between verify_area() and
> access_ok() (Jose added that code based on a suggestion from Linus, I
> think), but I believe access_ok() is intended as a security check, which
> is the reason for the copy. It seems that this all maps to the same thing
> for *BSD at the moment -- i.e. the unchecked macros aren't implemented
> differently from the checked ones, right?
They seem to be equivalent, onyle the semantics of the return value
differ. Here is the definition of verify_area in my distribution kernel
sources (linux-2.4.20-gentoo-r1), in include/asm-i386/uaccess.h:
static inline int verify_area(int type, const void * addr, unsigned long size)
{
return access_ok(type,addr,size) ? 0 : -EFAULT;
}
So perhaps we should converge on one.
> 2. The Mach64 driver makes heavy use of the list struct and macros from
> linux/list.h. I moved the define for list_for_each_safe() (needed for
> older 2.4 Linux kernels) from mach64_drv.h to drmP.h, since that has
> already been added in XFree86 CVS (I think the i8x0 drm uses it now also).
> I also removed the include of linux/list.h from the mach64 driver, since
> it already gets included (indirectly?) through the drm headers. However,
> it looks like an analogue of linux/list.h might need to be added to the
> BSD drm headers. The only wrinkle there is that it also uses
> linux/prefetch.h.
>
> 3. We still need to work out the wrapper/alternative to
> pci_alloc_consistent() and friends.
I'm still reading Ian texmem-2 proposal and looking to the source code
to get a hold of this.
> 4. As I mentioned before, the interrupt code is not converted, but it's
> currently unused.
My memory is failing: this might still be usefull for Xv, isn't it?
> At any rate, the remainder of the attached patch is trivial additions of
> DRM_ERR, DRM_CURRENTPID, etc. and a couple of whitespace tweaks.
José Fonseca
__
Do You Yahoo!?
Everything you'll ever need on one web page
from News and Sport to Email and Music Charts
http://uk.my.yahoo.com
---
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf
___
Dri-devel mailing list
[EMAIL PROTECTED]
https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
On Mon, 2003-02-17 at 17:15, Leif Delgass wrote: > As I was doing some minor cleanups in the mach64 drm in the new branch, I > made some additional search and replace conversions of the mach64 DRM to > the os independence macros (I couldn't restrain myself ;) ). However, I > want to share what I've done so far and get some feedback, since there are > a couple of issues here: > > 1. We are currently using access_ok() to check the vertex buffers > submitted by the client before copying them to (what will be) private > kernel buffers. There was only a macro in drm_os_linux.h for > DRM_VERIFYAREA_READ(), so I added DRM_ACCESSOK_READ(). I don't really > know the details of what the difference is between verify_area() and > access_ok() (Jose added that code based on a suggestion from Linus, I > think), but I believe access_ok() is intended as a security check, which > is the reason for the copy. It seems that this all maps to the same thing > for *BSD at the moment -- i.e. the unchecked macros aren't implemented > differently from the checked ones, right? Yeah, there's no unchecked versions for FreeBSD that I can find, and if it's in the middle of a loop it might be a pretty significant hit on cpu. > 2. The Mach64 driver makes heavy use of the list struct and macros from > linux/list.h. I moved the define for list_for_each_safe() (needed for > older 2.4 Linux kernels) from mach64_drv.h to drmP.h, since that has > already been added in XFree86 CVS (I think the i8x0 drm uses it now also). > I also removed the include of linux/list.h from the mach64 driver, since > it already gets included (indirectly?) through the drm headers. However, > it looks like an analogue of linux/list.h might need to be added to the > BSD drm headers. The only wrinkle there is that it also uses > linux/prefetch.h. I was wondering if we would want to bring in a stripped-down verion of BSD sys/queue.h. We could just have a subset of the tailq macros and add another macro to it for the foreach_safe. > 3. We still need to work out the wrapper/alternative to > pci_alloc_consistent() and friends. I'll get to work on this once we've got the lists issue cleared up. I think I understand the busdma stuff pretty decently at this point. > At any rate, the remainder of the attached patch is trivial additions of > DRM_ERR, DRM_CURRENTPID, etc. and a couple of whitespace tweaks. Looks good to me. -- Eric Anholt[EMAIL PROTECTED] http://people.freebsd.org/~anholt/ [EMAIL PROTECTED] --- This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven. http://thinkgeek.com/sf ___ Dri-devel mailing list [EMAIL PROTECTED] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: [Dri-devel] DRM OS Independence and Mach64
As I was doing some minor cleanups in the mach64 drm in the new branch, I
made some additional search and replace conversions of the mach64 DRM to
the os independence macros (I couldn't restrain myself ;) ). However, I
want to share what I've done so far and get some feedback, since there are
a couple of issues here:
1. We are currently using access_ok() to check the vertex buffers
submitted by the client before copying them to (what will be) private
kernel buffers. There was only a macro in drm_os_linux.h for
DRM_VERIFYAREA_READ(), so I added DRM_ACCESSOK_READ(). I don't really
know the details of what the difference is between verify_area() and
access_ok() (Jose added that code based on a suggestion from Linus, I
think), but I believe access_ok() is intended as a security check, which
is the reason for the copy. It seems that this all maps to the same thing
for *BSD at the moment -- i.e. the unchecked macros aren't implemented
differently from the checked ones, right?
2. The Mach64 driver makes heavy use of the list struct and macros from
linux/list.h. I moved the define for list_for_each_safe() (needed for
older 2.4 Linux kernels) from mach64_drv.h to drmP.h, since that has
already been added in XFree86 CVS (I think the i8x0 drm uses it now also).
I also removed the include of linux/list.h from the mach64 driver, since
it already gets included (indirectly?) through the drm headers. However,
it looks like an analogue of linux/list.h might need to be added to the
BSD drm headers. The only wrinkle there is that it also uses
linux/prefetch.h.
3. We still need to work out the wrapper/alternative to
pci_alloc_consistent() and friends.
4. As I mentioned before, the interrupt code is not converted, but it's
currently unused.
At any rate, the remainder of the attached patch is trivial additions of
DRM_ERR, DRM_CURRENTPID, etc. and a couple of whitespace tweaks.
--
Leif Delgass
http://www.retinalburn.net
Index: linux/drm/kernel/drmP.h
===
RCS file:
/cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h,v
retrieving revision 1.56
diff -u -r1.56 drmP.h
--- linux/drm/kernel/drmP.h 11 Jan 2003 20:58:20 - 1.56
+++ linux/drm/kernel/drmP.h 18 Feb 2003 00:33:29 -
@@ -166,6 +166,12 @@
#define pte_unmap(pte)
#endif
+#ifndef list_for_each_safe
+#define list_for_each_safe(pos, n, head) \
+ for (pos = (head)->next, n = pos->next; pos != (head); \
+ pos = n, n = pos->next)
+#endif
+
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,4,19)
static inline struct page * vmalloc_to_page(void * vmalloc_addr)
{
Index: linux/drm/kernel/drm_os_linux.h
===
RCS file:
/cvsroot/dri/xc/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_os_linux.h,v
retrieving revision 1.5
diff -u -r1.5 drm_os_linux.h
--- linux/drm/kernel/drm_os_linux.h 9 Oct 2002 16:29:01 - 1.5
+++ linux/drm/kernel/drm_os_linux.h 18 Feb 2003 00:33:30 -
@@ -34,6 +34,8 @@
/* Macros for copyfrom user, but checking readability only once */
#define DRM_VERIFYAREA_READ( uaddr, size ) \
verify_area( VERIFY_READ, uaddr, size )
+#define DRM_ACCESSOK_READ( uaddr, size ) \
+ access_ok( VERIFY_READ, uaddr, size )
#define DRM_COPY_FROM_USER_UNCHECKED(arg1, arg2, arg3) \
__copy_from_user(arg1, arg2, arg3)
#define DRM_GET_USER_UNCHECKED(val, uaddr) \
@@ -60,28 +62,28 @@
#define DRM_HZ HZ
-#define DRM_WAIT_ON( ret, queue, timeout, condition ) \
-do { \
- DECLARE_WAITQUEUE(entry, current); \
- unsigned long end = jiffies + (timeout);\
- add_wait_queue(&(queue), &entry); \
- \
- for (;;) { \
- current->state = TASK_INTERRUPTIBLE;\
- if (condition) \
- break; \
- if((signed)(end - jiffies) <= 0) { \
- ret = -EBUSY; \
- break; \
- } \
+#define DRM_WAIT_ON( ret, queue, timeout, condition ) \
+do { \
+ DECLARE_WAITQUEUE(entry, current); \
+ unsigned long end = jiffies + (timeout);\
+ add_wait_queue(&(queue), &entry); \
+ \
+ for (;;) { \
+ current->state =
