dell / dkms

Dynamic Kernel Module Support
GNU General Public License v2.0
631 stars 149 forks source link

Kernel match for BUILT_MODULE_NAME #392

Open dmipx opened 7 months ago

dmipx commented 7 months ago

We are building UVC kernel module. To rebuild uvc modules prior to kernel 6.5 we needed only videodev and uvcvideo modules. With kernel 6.5 there was refactoring of uvc framework and now we have another dependency - uvc module.

modinfo  /lib/modules/6.5.0-15-generic/kernel/drivers/media/usb/uvc/uvcvideo.ko | grep depends
depends:        videobuf2-v4l2,videodev,mc,uvc,videobuf2-common,videobuf2-vmalloc

Kernel 5.15, same as 6.2:

modinfo /lib/modules/5.15.0-25-generic/kernel/drivers/media/usb/uvc/uvcvideo.ko | grep depends
depends:        videobuf2-v4l2,videodev,videobuf2-common,videobuf2-vmalloc,mc

Getting this error for building on kernel 5.15.0 when having another module in conf file: BUILT_MODULE_NAME[2]="uvc" Error! Build of uvc.ko failed for: 5.15.0-25-generic (x86_64)

What is missing now is key DEPENDS, like BUILT_MODULE_DEPENDS:

BUILT_MODULE_NAME[1]="videodev"
BUILT_MODULE_LOCATION[1]="drivers/media/v4l2-core/"
DEST_MODULE_NAME[1]="videodev"
DEST_MODULE_LOCATION[1]="/kernel/drivers/media/v4l2-core/"

# Kernel 6.5
BUILT_MODULE_NAME[2]="uvc"
BUILT_MODULE_LOCATION[2]="drivers/media/common/"
BUILT_MODULE_DEPENDS[2]="^(6.[5])"
DEST_MODULE_NAME[2]="uvc"
DEST_MODULE_LOCATION[2]="/kernel/drivers/media/common/"
DEST_MODULE_DEPENDS[2]="^(6.[5])"

Thanks.

dmipx commented 7 months ago

CLEAN[#] and CLEAN_MATCH[#] can be additional improvement.

evelikov commented 7 months ago

We recently got some options that may help here: BUILD_EXCLUSIVE_KERNEL_MIN and ..._MAX variant.

Both of those are global, meaning per dkms.conf module, instead of per kernel module.

At a glance, two ways to move forward come to mind:

dmipx commented 7 months ago

Thanks for tips. That means we need to maintain two DKSM packages - one for kernels below 6.5 and other for kernels above 6.5. Our delivery is debian dpkg packaged DKMS, I cannot see such option to have both in one bundle..

evelikov commented 7 months ago

Yes for option A two packages will be needed. That should be practically zero maintenance (given some minor automation), since a) the code will be identical and b) the pre 6.5 dkms.conf will effectively be frozen.

Assuming you scope the Debian versions, you can even get away with single package.

If that's not a route you prefer, option B should work. Although for you'd need to write a bit of code and tests ;-)

Aside: you'd want to try modinfo -F depends /lib/modules/5.15.0-25-generic/kernel/drivers/media/usb/uvc/uvcvideo.ko, over the manual grepping.

anbe42 commented 7 months ago

One of the most sophisticated (and dynamically self-generating) dkms.conf I'v seen so far is https://sources.debian.org/src/lttng-modules/2.13.11-1/debian/lttng-modules-dkms.dkms/?hl=1#L1

But for your problem its probably sufficient to do something conditional like

if <<<CONDITION>>> ; then
BUILT_MODULE_NAME[2]="uvc"
...
fi

(you have the complete bash syntax available for <<<CONDITION>>>)

dmipx commented 7 months ago

Excellent! I forgot that conf file is also bash syntax.

dmipx commented 7 months ago

Resolved that with

# Kernel 6.5 introduced uvc module
if [ "$VERSION" -ge 6 -a "$PATCHLEVEL" -ge 5 ]; then
    BUILT_MODULE_NAME[2]="uvc"
    BUILT_MODULE_LOCATION[2]="drivers/media/common/"
    DEST_MODULE_NAME[2]="uvc"
    DEST_MODULE_LOCATION[2]="/kernel/drivers/media/common/"
fi

Issue can be closed.

anbe42 commented 7 months ago

you probably want (the bash equvivalent of) (V >= 7) || (V == 6 && P >= 5) otherwise this will not catch e.g. 7.2

evelikov commented 7 months ago

Personally I find using bash/shell scripting in the configuration file an abuse of implementation details. In the past we have taken actions to reduce that and I expect more to come in the future.

So be warned ;-)

Let's keep this open with the goal of making BUILD_EXCLUSIVE_KERNEL_MIN/MAX an array.