An issue raised by DRM people with the new drm core is how to stop users
shotting themselves in the foot when upgrading drm modules from CVS and
mixing up cores and drivers...
This patch (against DRM CVS) proposes a simple internal version that gets
passed from the module to the core, when built in-kernel, it gets set to
default DRM_INTERNAL_VERSION_KERNEL, when built in DRM CVS or snapshot, it
gets DRM_INTERNAL_VERSION_EXTERNAL, a core built in one will refuse to
load a module build in the other..
This is quite a simple solution that should stop the most obvious issue,
it doesn't stop people updating CVS drivers on top of themselves but my
view is anyone doing this is either following our scripts or knows what
they are doing...
Dave.
Index: linux-core/Makefile
===================================================================
RCS file: /cvs/dri/drm/linux-core/Makefile,v
retrieving revision 1.17
diff -u -r1.17 Makefile
--- linux-core/Makefile 30 Sep 2004 20:25:13 -0000 1.17
+++ linux-core/Makefile 9 Oct 2004 23:48:31 -0000
@@ -282,6 +282,8 @@
# This needs to go before all other include paths.
CC += -I$(DRMSRCDIR)
+EXTRA_CFLAGS = -DDRM_INTERNVAL_VERSION=DRM_INTERNAL_VERSION_EXTERNAL
+
# Check for Red Hat's 4-argument do_munmap().
DOMUNMAP := $(shell grep do_munmap $(LINUXDIR)/include/linux/mm.h | \
grep -c acct)
Index: linux-core/drmP.h
===================================================================
RCS file: /cvs/dri/drm/linux-core/drmP.h,v
retrieving revision 1.130
diff -u -r1.130 drmP.h
--- linux-core/drmP.h 9 Oct 2004 10:58:19 -0000 1.130
+++ linux-core/drmP.h 9 Oct 2004 23:48:35 -0000
@@ -87,6 +87,13 @@
#include "drm_os_linux.h"
+#define DRM_INTERNAL_VERSION_KERNEL 1
+#define DRM_INTERNAL_VERSION_EXTERNAL 2
+
+#ifndef DRM_INTERNAL_VERSION
+#define DRM_INTERNAL_VERSION DRM_INTERNAL_VERSION_KERNEL
+#endif
+
/* If you want the memory alloc debug functionality, change define below */
/* #define DEBUG_MEMORY */
@@ -730,7 +737,8 @@
extern int drm_fb_loaded;
extern int __devinit drm_init(struct pci_driver *driver,
struct pci_device_id *pciidlist,
- struct drm_driver_fn *driver_fn);
+ struct drm_driver_fn *driver_fn,
+ int internal_version);
extern void __exit drm_exit(struct pci_driver *driver);
extern void __exit drm_cleanup_pci(struct pci_dev *pdev);
extern int drm_version(struct inode *inode, struct file *filp,
Index: linux-core/drm_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/drm_drv.c,v
retrieving revision 1.96
diff -u -r1.96 drm_drv.c
--- linux-core/drm_drv.c 8 Oct 2004 14:31:25 -0000 1.96
+++ linux-core/drm_drv.c 9 Oct 2004 23:48:37 -0000
@@ -74,6 +74,8 @@
#undef DRM_OPTIONS_FUNC
#endif
+static int drm_internal_version=DRM_INTERNAL_VERSION;
+
int drm_fb_loaded = 0;
/** Ioctl table */
@@ -320,7 +322,7 @@
*/
int drm_init(struct pci_driver *driver,
struct pci_device_id *pciidlist,
- struct drm_driver_fn *driver_fn)
+ struct drm_driver_fn *driver_fn, int internal_version)
{
struct pci_dev *pdev;
struct pci_device_id *pid;
@@ -332,6 +334,11 @@
drm_parse_options(drm_opts);
#endif
+ if (drm_internal_version!=internal_version) {
+ printk("Attempt to load DRM module on different core\m");
+ return -1;
+ }
+
drm_mem_init();
for (i = 0; (pciidlist[i].vendor != 0) && !drm_fb_loaded; i++) {
Index: linux-core/i810_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/i810_drv.c,v
retrieving revision 1.46
diff -u -r1.46 i810_drv.c
--- linux-core/i810_drv.c 30 Sep 2004 21:12:05 -0000 1.46
+++ linux-core/i810_drv.c 9 Oct 2004 23:48:37 -0000
@@ -132,7 +132,7 @@
static int __init i810_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit i810_exit(void)
Index: linux-core/i830_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/i830_drv.c,v
retrieving revision 1.13
diff -u -r1.13 i830_drv.c
--- linux-core/i830_drv.c 30 Sep 2004 21:12:05 -0000 1.13
+++ linux-core/i830_drv.c 9 Oct 2004 23:48:37 -0000
@@ -142,7 +142,7 @@
static int __init i830_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit i830_exit(void)
Index: linux-core/i915_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/i915_drv.c,v
retrieving revision 1.7
diff -u -r1.7 i915_drv.c
--- linux-core/i915_drv.c 30 Sep 2004 21:12:05 -0000 1.7
+++ linux-core/i915_drv.c 9 Oct 2004 23:48:37 -0000
@@ -106,7 +106,7 @@
static int __init i915_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit i915_exit(void)
Index: linux-core/mach64_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/mach64_drv.c,v
retrieving revision 1.7
diff -u -r1.7 mach64_drv.c
--- linux-core/mach64_drv.c 30 Sep 2004 21:12:05 -0000 1.7
+++ linux-core/mach64_drv.c 9 Oct 2004 23:48:37 -0000
@@ -123,7 +123,7 @@
static int __init mach64_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit mach64_exit(void)
Index: linux-core/mga_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/mga_drv.c,v
retrieving revision 1.44
diff -u -r1.44 mga_drv.c
--- linux-core/mga_drv.c 30 Sep 2004 21:12:06 -0000 1.44
+++ linux-core/mga_drv.c 9 Oct 2004 23:48:37 -0000
@@ -128,7 +128,7 @@
static int __init mga_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit mga_exit(void)
Index: linux-core/r128_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/r128_drv.c,v
retrieving revision 1.51
diff -u -r1.51 r128_drv.c
--- linux-core/r128_drv.c 30 Sep 2004 21:12:06 -0000 1.51
+++ linux-core/r128_drv.c 9 Oct 2004 23:48:37 -0000
@@ -139,7 +139,7 @@
static int __init r128_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit r128_exit(void)
Index: linux-core/radeon_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/radeon_drv.c,v
retrieving revision 1.25
diff -u -r1.25 radeon_drv.c
--- linux-core/radeon_drv.c 30 Sep 2004 21:12:06 -0000 1.25
+++ linux-core/radeon_drv.c 9 Oct 2004 23:48:40 -0000
@@ -179,7 +179,7 @@
static int __init radeon_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit radeon_exit(void)
Index: linux-core/savage_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/savage_drv.c,v
retrieving revision 1.10
diff -u -r1.10 savage_drv.c
--- linux-core/savage_drv.c 30 Sep 2004 21:12:06 -0000 1.10
+++ linux-core/savage_drv.c 9 Oct 2004 23:48:40 -0000
@@ -316,7 +316,7 @@
static int __init savage_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit savage_exit(void)
Index: linux-core/sis_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/sis_drv.c,v
retrieving revision 1.23
diff -u -r1.23 sis_drv.c
--- linux-core/sis_drv.c 30 Sep 2004 21:12:06 -0000 1.23
+++ linux-core/sis_drv.c 9 Oct 2004 23:48:40 -0000
@@ -105,7 +105,7 @@
static int __init sis_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit sis_exit(void)
Index: linux-core/tdfx_drv.c
===================================================================
RCS file: /cvs/dri/drm/linux-core/tdfx_drv.c,v
retrieving revision 1.42
diff -u -r1.42 tdfx_drv.c
--- linux-core/tdfx_drv.c 30 Sep 2004 21:12:06 -0000 1.42
+++ linux-core/tdfx_drv.c 9 Oct 2004 23:48:40 -0000
@@ -96,7 +96,7 @@
static int __init tdfx_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit tdfx_exit(void)
Index: shared-core/via_drv.c
===================================================================
RCS file: /cvs/dri/drm/shared-core/via_drv.c,v
retrieving revision 1.13
diff -u -r1.13 via_drv.c
--- shared-core/via_drv.c 9 Oct 2004 12:42:52 -0000 1.13
+++ shared-core/via_drv.c 9 Oct 2004 23:48:40 -0000
@@ -113,7 +113,7 @@
static int __init via_init(void)
{
- return drm_init(&driver, pciidlist, &driver_fn);
+ return drm_init(&driver, pciidlist, &driver_fn, DRM_INTERNAL_VERSION);
}
static void __exit via_exit(void)
How strong of match requirement do we need? Note that this only
impacts distribution of binary personality modules, if you have source
there is no problem.
Several stronger solutions:
on each CVS check-in to DRM increment a count and use this for an id
manual version number for DRM
embed the kernel version
embed the kernel version plus distribution info
--
Jon Smirl
[email protected]
> How strong of match requirement do we need? Note that this only
> impacts distribution of binary personality modules, if you have source
> there is no problem.
Not really I'm thinking more of someone building a module against one core
and insmodding it against another one.. so someone builds a kernel with
core/personality, then builds just a personality module from CVS and tries
to use it with the kernel core one...
personally I think binary distributors have the money to keep up with the
kernel releases....
I don't want to re-implement kernel modversions which is what we are close
to doing, you can't insmod a module built against a different kernel
anyways so it doesn't matter, kernel version, preempt, smp, compiler are
all checked on insmod in 2.6 if they don't match it doesn't load it is not
possible to distrib a binarry kernel independent module.. without at least
a portable stub source loader...
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person
On Sun, Oct 10, 2004 at 03:31:26AM +0100, Dave Airlie wrote:
>
> I don't want to re-implement kernel modversions which is what we are close
> to doing
Then why not just rely on the modversion code in the kernel tree to do
this? As you say:
> you can't insmod a module built against a different kernel
> anyways so it doesn't matter, kernel version, preempt, smp, compiler are
> all checked on insmod in 2.6 if they don't match it doesn't load it is not
> possible to distrib a binarry kernel independent module..
Which is a pretty good reason not to try to implement your own
versioning system, right?
> without at least a portable stub source loader...
Are you thinking that someone will try to do this? If so, they deserve
what they get :)
thanks,
greg k-h
>
> Then why not just rely on the modversion code in the kernel tree to do
> this? As you say:
well that's what I hope to do, I'd just like to stop people doing the
basic silly thing of insmodding a personality on a core built from a
different tree...
the main reason I can't rely on CONFIG_MODVERSIONS is in my Fedora install
at least: CONFIG_MODVERSIONS is not set so it is of no use if it is
optional...
> Which is a pretty good reason not to try to implement your own
> versioning system, right?
excactly that's why I just want to do the simple thing that I posted as
opposed to Jons scheme that increases version numbers and things
depending on builds... I don't think we need that, the problem we are
trying to solve is that of someone taking an external DRM tree, building
it and trying to only use the personality modules.. for us this is a
support issue we deal with the mails.. someone like ATI is still free for
example to do it, they can build their module against the current kernel
core code and all will be okay... and if there is any issues they are paid
to deal with it...
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person
>
> the main reason I can't rely on CONFIG_MODVERSIONS is in my Fedora install
> at least: CONFIG_MODVERSIONS is not set so it is of no use if it is
> optional...
The versioning we all talk about doesn't use MODVERSIONS but the
VERSIONMAGIC stuff, that is ALWAYS in use.
>
> The versioning we all talk about doesn't use MODVERSIONS but the
> VERSIONMAGIC stuff, that is ALWAYS in use.
No I'm actually talking about both :-)
VERSIONMAGIC stops the using binary modules and now that I think off it,
I'm not sure what great use MODVERSIONS have anymore if you can't load a
module into a different kernel version...
Dave.
--
David Airlie, Software Engineer
http://www.skynet.ie/~airlied / airlied at skynet.ie
pam_smb / Linux DECstation / Linux VAX / ILUG person
Dave Airlie wrote:
> An issue raised by DRM people with the new drm core is how to stop users
> shotting themselves in the foot when upgrading drm modules from CVS and
> mixing up cores and drivers...
>
> This patch (against DRM CVS) proposes a simple internal version that gets
> passed from the module to the core, when built in-kernel, it gets set to
> default DRM_INTERNAL_VERSION_KERNEL, when built in DRM CVS or snapshot, it
> gets DRM_INTERNAL_VERSION_EXTERNAL, a core built in one will refuse to
> load a module build in the other..
>
> This is quite a simple solution that should stop the most obvious issue,
> it doesn't stop people updating CVS drivers on top of themselves but my
> view is anyone doing this is either following our scripts or knows what
> they are doing...
I guess I don't get it. We want to prevent a DRM module that requires
core API N with a drm_core that exports M, M != N, right? So why not do
that? In that model don't you just need DRM_INTERNAL_VERSION? With the
setup you propose it seems like we could still have problems. If a user
installs a snapshot, then installs just a DRM from a later snapshot, etc.