The patch that will be attached below adds support for 32bit clients on 64bit platforms. The patch has been developed and tested on AMD64 with the Radeon, R128 and MGA drivers. The patch mainly does three things: 1. It modifies data structures that are passed between a 64bit server and clients (that can be 64 or 32 bit) to contain only data types that have the same size in 32 and 64bit. 2. It attempts to hide the drm_handle_t type from the clients. This type is used for mapping handles that are passed between kernel, Xserver and clients. For convenience reasons the kernel space addresses of the mapped areas are used which confines this type to unsigned long. However a 64bit unsigned long cannot be used in a 32bit mmap. The new code calculates a 32bit unsigned int hash value which identifes the memory area. Some dri X drivers (at least in the past) took advantage of the fact that these handles are physical addresses in some cases. This has been fixed in the Matrox driver but may still be a problem for the drivers that have not been tested. 3. It adds ioct32 handlers for those IOCTL structures that contain data types that have different size and/or alingment in 32 and 64bit. Currently the modifications have only been done for Linux. It is expected that considerable work is required for other platforms that use DRM.
Created attachment 540 [details] [review] Patch described in previous comment.
Created attachment 541 [details] [review] Oops. Fixed patch Previous patch was missing pieces. Sorry.
*** Bug 942 has been marked as a duplicate of this bug. ***
Reassigning to dri-devel.
Comments?
I expect that the code no longer applies to the current CVS version. I guess I need to port it again. Furthermore I was going to look into making the backward compatibility handling cleaner.
The big issue is backwards compatiblity, this patch needs to be developed as two separate patches, one for the kernel, then tested with current X releases and submitted via the DRM tree, then one for Mesa and X that can also work with the old drm on normal systems (this isn't so important, you can get users to upgrade their kernel if they are upgrading X just use the DRM versioning, but upgrading your kernel should never break your X/DRI)...
I also meant to say that if for some reason this is impossible, then these reasons should be raised with the kernel/X people and we find the solution that breaks the least number of working systems.... My belief at the moment (with no 64-bit machines to validate anything on) is that anything we do will probably break pure 64-bit clients, breaking pure 32-bit clients is of course completely unacceptable.... but maybe someone can find a way...
Any updates on this yet ? A patch against the new drm would be fine. Greetings Jan
I haven't had time to do this, yet. It's on my list.
Created attachment 2011 [details] [review] Patch to add ioctl compatibility routines for 32-bit processes Here is my attempt at implementing ioctl compatibility routines for 32-bit processes on a 64-bit kernel. The patch is against a 2.6.11 Linux kernel tree. I have done the core DRM and the Radeon driver but not any of the other chipset drivers. I haven't changed the ABI, either for 32-bit or for 64-bit processes. I have used this successfully on a G5 powermac running a ppc64 Linux kernel, using unmodified 32-bit X server and client binaries.
Hi i used the patch by Paul Mackerras on an x86-64 machine running Debian-Sid with Xorg from Ubuntu (with latest xorg-cvs ati drivers) and r300_driver from r300.sf.net. I tested it on an Pure ia32 setup and on my x86-64 setup. I tested with glxgears :) and neverball. It works for me in an pure ia32-setup, in an pure x86-64 setup. It also works when i boot an x86-64 kernel and chroot into my ia32-setup (as i think this is similiar to Pauls setup this was to be expspected). However when i boot into x86-64 and start and x86-64 Xserver and then chroot into my ia32-setup every gl-related operation (glxinfo, neverball ...) segfault. It would be nice, if i could start an x86-64 Xserver and use 32-bit gl-apps (as it is possible with fglrx), however currently i am very happy with the solution to stop my x86-64 Xserver, chroot into my ia32-setup and start the ia32 Xserver (as this was mostly missing using dri on x86-64). If you need more Information (eg on the segfault, or an patch against r300.sf.net drm sources or ...) feel free to ask. Thanks for your work. cheers Jan Kreuzer
Created attachment 2024 [details] dmesg from 64-bit kernel and Xserver and 32-bit glxinfo This is my dmesg output from an X86-64 kernel and Xserver and an ia32 chrooted glxinfo.
Created attachment 2025 [details] strace from 32-bit glxinfo on 64-bit kernel and Xserver This is the strace from an chrooted ia32 glxinfo on an X86-64 Kernel and Xserver.
Hmmm, presumably the X server is doing the addmap for the SAREA and then the client is mmapping it, but is using the wrong offset value. I see that the SAREA handle gets sent from the X server to the client in XF86DRIOpenConnection. I think the way to solve the problem is to always make a 32-bit handle for _DRM_SHM memory when CONFIG_COMPAT is defined, so that the X server will get a 32-bit handle for the SAREA even if it is a 64-bit process.
> I think the way to solve the problem is to always make a 32-bit handle for > _DRM_SHM memory when CONFIG_COMPAT is defined, so that the X server will get a > 32-bit handle for the SAREA even if it is a 64-bit process. That's exactly what my code is doing. I've got an updated version of my patch that handles backward compatibility much more gracefully.
In comment #11 Paul Mackerras writes: > I haven't changed the ABI, either for 32-bit or for 64-bit processes. I have > used this successfully on a G5 powermac running a ppc64 Linux kernel, using > unmodified 32-bit X server and client binaries. Unfortunately without a change in the ABI it is not possible to run 32bit DRI clients on 64 bit servers (or vice versa). The problem is that data structures shared between kernel, Xserver and clients have different sizes. However it is conceivable that users may want to run 32 and 64 applications on the same Xserver. With the reight defines the ioctl converters are very boiler plate. I'm going to attach an updated version of my patch. Presently I have support to the MGA, Radeon and Rage128 driver. The changes to DRM should be backward compatible so that unpatched Xservers and DRI libraries continue to work. Xserver, libGL and DRI drivers need to be updated together however.
Created attachment 2362 [details] [review] Updated patch (DRM part) This patch changes DRM so that 32bit DRI clients work on a 64bit kernel. It should be backward compatible to older versions of Xservers and client libraries.
Created attachment 2363 [details] [review] Fix for Mesa DRI drivers. This patch fixes the Mesa DRI drivers to support 32bit DRI clients on 64bit systems. It requires the patch in attachment #2362 [details] [review] and the Xserver patch that is going to be attached.
Created attachment 2364 [details] [review] Fix for Xserver part. This patch makes the required adjustments to the Xserver that are required when using attachment #2362 [details] [review] and #2363. The patch applies to a recent version of X.Org head. I have not bothered to apply all patches from #2363 to extra/Mesa also.
I still don't see any reason why the ABI needs to be changed. The comment "data structures shared between kernel, Xserver and clients have different sizes" is too vague to be useful - which data structures, and what stops them being converted?
> I still don't see any reason why the ABI needs to be changed. The comment > "data structures shared between kernel, Xserver and clients have different > sizes" is too vague to be useful - which data structures, and what stops them > being converted? Let me give you two examples: 1. mga_dri.h MGADRIRec is a data structure that is private to the Matrox DRI driver. It gets initialized in the DRI X driver. A DRI client (better to say the dri client module) can obtain this data structure from the Xserver thru the DRI extension request X_XF86DRIGetDeviceInfo. Usually X Request structures are carefully designed to be machine independent. Not in this case however: Since DRI assumes that communication happens locally it simply copies over the entire structure. If you look at the above structure it contains drmRegion elements. drmRegion contains a pointer and this is 32bit on a 32bit system or for a 32bit client on a 64bit system and 64bit if the binary is 64 bit. Therefore the last element of the MGADRIRec sarea_priv_offset comes at a different offset depending if your client is 32 or 64 bit. If you connect to a 64bit Xserver from a 32bit client your sarea_priv_offset value read by the client is probably wrong. Why should do the conversion? There is no way for the client to know if the structure has been sent by a 32bit or 64bit server. Also the server doesn't know what client is connected. (Only endianess information is transmitted in the X connection block). 2. The SAREA also contains a driver private structure (in case of the matrox driver contained in mga_sarea.h:MGASAREAPrivRec (drm: mga_drm.h:drm_mga_sarea_t). Its data is shared between the kernel and a DRI client. On 64bit systems it's conceivable that the kernel runs 64bit. How can the kernel and a DRI client share data thru a structure with different size? Converting the data structures of the ioctls is the easy part. You can simply write 'wrappers'. The only exception is where the kernel hands over 64bit 'handles' to user space which are used as offsets in mmap(). This doesn't work in 32bit user space. I've solved this problem without changing the ABI. However the handle is no real pointer any more which could cause issues to some drivers.
On the DRM portion can you look at the work Paul has done, it is much cleaner and less intrusive, you are also using the old method which is deprecated now in the kernel, the new method is available in Pauls patch in the -mm kernel, maybe Paul can generate a patch against 2.6.11.... My feeling on this now is that we should change to generating hashes for SHM handles for *all* platforms, having it different on 32 and 64 bits isn't going to help thing, make it break obviously for everyone and we'll find out if we can use this method or not ... I'd like to merge the DRM into the kernel and see who complains and maybe provide a command line option to fall back to the old 32-bit handle for 32-bit systems in case somebody breaks badly... I think between the two of you a clean solution should be possible, breaking the ABI between X and DRI clients isn't as bad as breaking the kernel/userspace boundard but I'd like to keep the 32-bit ABI the same if possible but I guess it might not be, I've no issues with telling someone to upgrade X and DRI at the same time from snapshots..
After a chat on IRC perhaps we can define a new DRIRec and just up the DDX numbers for the affected drivers.. then clients can know new/old DDX and either just use the new structure or do either of fail/fixup for the old structure depending on what they are running on.
OK, Egbert is right concerning the server -> client communication. I ran across this with RADEONDRIRec. I checked all the SAREA definitions and they all seem to be OK, fortunately. It may be possible to convert in the client (based on the size of the structure received from the server) so that a new client can run with either a 32-bit or 64-bit server. Longer term I agree that changing the RADEONDRIRec and other structures to be wordsize-independent is a good idea.
One could do a conversion - on the other hand a brakage of the ABI compatibility between client and Xserver would not be all that bad - given that we cannot really distinguish if a 32bit client library contains code that fixes the size or not. The main issue that I ran into was the size of drm_handle_t which I changed from 'long' to 'int'. It's defined in the drm headers however the drm modules don't seem to use it at all. It ensures that the same handles are used in 32 and 64bit and can be exchanged between 32 and 64bit pieces. This is done between the Xserver and 32bit clients. This however requires that 32 and 64bit both return 32bit handles. This may be the reason why my code is more intrusive. Apart from this of all the drivers I fixed I only saw an issue with Matrox. > On the DRM portion can you look at the work Paul has done, it is much cleaner > and less intrusive, you are also using the old method which is deprecated now > in > the kernel, the new method is available in Pauls patch in the -mm kernel, > maybe > Paul can generate a patch against 2.6.11.... The kernel ABI really hasn't changed. The ioctls have remained the same. The only change I've made was to make sure the handles passed between kernel and user space fit into 32bit. That means that they are not necessarily real offset values any more. If anything needs real offset values it needs to obtain it thru another way. If fit in 32bits my code attempts to reuse them. If they are 64bit however a 32bit hash is created. It is also make sure that the handles are unique. For your argument about intrusiveness please see above. I'm presently reworking my driver to support both the old and the new method. My intention was to make the creation of 32bit ioctls as boiler plate as possible. My hope was that this would encourage people to provide compat support for every driver.
Created attachment 2418 [details] [review] Patch for DRI client / X server compatibility This is what I am using at the moment, and with this, both 32-bit and 64-bit clients can connect to either a 32-bit or a 64-bit X server and do direct rendering. I have tested the r300 part of this patch on my G5. I have made the same changes to radeon and r200 and compile-tested them, but I don't have a 64-bit machine with a r100 or r200 graphics card. I agree that the drm_handle_t is the main problem. As for changing the interface, I think we should do that when the next reasonable opportunity arises. But for now, something like this would let us fix the 32/64 problem without breaking anything that currently works.
A comment w.r.t. generating hashed handles: there are some cases where the radeon family of client-side DRI drivers do arithmetic to generate handles. I *think* this is only for AGP memory. I have a suspicion that the X server may also generate handles for the registers and/or the framebuffer by reading the device's PCI bus address registers, but I would need to verify that. Anyway, the point is that the handles can't be completely arbitrary for all types of mappings, at least not without checking a lot of code...
> A comment w.r.t. generating hashed handles: there are some cases where the > radeon family of client-side DRI drivers do arithmetic to generate handles. I > *think* this is only for AGP memory. I have a suspicion that the X server may > also generate handles for the registers and/or the framebuffer by reading the > device's PCI bus address registers, but I would need to verify that. This is totally broken behavior. It is very poor design. I have encountered such cases (but I don't think the Radeon driver contained any of them - however the MGA driver did.) I fixed all of them. Fortunately there were not too many. There are better ways to obtain the information that is needed than to rely on the handle to contain a certain piece of information. > Anyway, the point is that the handles can't be completely arbitrary for all > types of mappings, at least not without checking a lot of code... Well, for some mappings we may be able to pass on the old values (if they fit into 32 bit) This is what my latest code does. It still produces ambiguities which need to be resolved. I'm going to attach a patch to my latest version of the code that addresses some security problems. The code can be compied to use either the old or new ioctl32 sceme.
Created attachment 2689 [details] [review] Patch on top of 2362 This patch adds support for both the old and the new ioctl32 sceme and fixes a security issue, a potential (yet unlikely) race condition. Furthermore it attempts to return handles that fit into 32bit unmodified. Client side code that uses the returned values for calculations should be fixed - nevertheless this change should work around these problems. This code has been tested with Radeon (in a modified tree as CVS head doesn't work on my cards), R128 and Matrox.
Created attachment 2691 [details] [review] Full patch (combining attachment #2362 [details] [review] and #2689) To build the code without using compat_alloc_user_space() add the define OLD_COMPAT.
Code is now in DRM CVS. Closing.
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.