Page MenuHomeFreeBSD

graphics/drm-current-kmod: Install kernel module sources.
ClosedPublic

Authored by jhb on Jul 18 2019, 9:55 PM.

Details

Summary

Install kernel module sources and makefiles to
$PREFIX/sys/modules/drm-current-kmod. This permits kernel builds to build
the DRM modules as part of buildkernel via the LOCAL_MODULES framework.
These modules are installed to the kernel's directory and are then
preferred to the generic modules installed in /boot/modules.

Test Plan
  • Built a custom kernel and installed it to /boot/test after installing an updated package. The kernel in /boot/test includes DRM kernel modules built from these sources.

I did not bump PORTREVISION since this shouldn't affect the binaries,
but that can be added if it is deemed important.

Diff Detail

Repository
rP FreeBSD ports repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jhb created this revision.Jul 18 2019, 9:55 PM
jhb added inline comments.Jul 18 2019, 10:04 PM
graphics/drm-current-kmod/Makefile
64 ↗(On Diff #59901)

It may be that Uses/kmod.mk should define this and the corresponding PLIST_SUB since I would expect other ports that install kernel module sources to follow the same convention?

75 ↗(On Diff #59901)

Using MAKEOBJDIRPREFIX avoided putting object files in the source directories preventing plist errors when the build directories are copied in the post-install target.

graphics/drm-current-kmod/pkg-message
8 ↗(On Diff #59901)

You don't need /boot/modules on HEAD where DRM2 isn't built and installed by default anymore, and once you build a kernel that installs its own DRM modules you need this variant instead for it to work properly. I think 12.0 also ships without DRM2, so we could perhaps makes this a pkg-message.in file that picks different versions based on the OS version (e.g. use this format for 12.0 and later and the /boot/modules one for older ones).

Thank you for working on this!

You have added all the sources to the pkg-plist, but you don't always install all the sources, it's dependent on the architecture. This works on amd64, since all sources are used there, but not on other platforms.

Once we've got this somewhere we're happy, we should probably get buy-in from portmgr as well.

graphics/drm-current-kmod/Makefile
64 ↗(On Diff #59901)

Yes, it would be nice if this is handled by the kmod infrastructure. We can invent our own for now though, until more ports get on this though.

70 ↗(On Diff #59901)

At line 64 you set KMODSRC, but here you use something else, why?

75 ↗(On Diff #59901)

I believe this should eventually be handled by the infrastructure as well.

82 ↗(On Diff #59901)

You might need to check/set owner/permission of the files here as well.

graphics/drm-current-kmod/pkg-message
8 ↗(On Diff #59901)

This is probably a left over for when the drm2 kmods where in current as well. For 12.0, and indeed the entire 12 branch, the kmods are shipped as part of base. We tried to get rid of them before 12 branched, but there was too much push back on that, and we had to revert that change. For the stable branches, we also have to work around the fact that with this, we would build several kmods with the same name, that is probably not a good idea.

drm-current-kmod is only used by CURRENT, we can get that working first. It's on current where the need to recompile the modules with each kernel is the biggest, and think about STABLE once we get this working.

jhb added a comment.Jul 20 2019, 4:28 PM

Thank you for working on this!
You have added all the sources to the pkg-plist, but you don't always install all the sources, it's dependent on the architecture. This works on amd64, since all sources are used there, but not on other platforms.

Hmm, I thought I matched what I do. I always install 'drivers' and 'include' which contains the bulk of the sources. The only conditional bits are the subdirectories that contain the module Makefiles, and those
are conditional (e.g. i915/Makefile)

Once we've got this somewhere we're happy, we should probably get buy-in from portmgr as well.

That's fine with me.

graphics/drm-current-kmod/Makefile
70 ↗(On Diff #59901)

For the PLIST_SUB it has to be a relative path that doesn't include STAGEDIR and PREFIX.

82 ↗(On Diff #59901)

Hmm, possibly. I'm not sure of the best way to handle that and will defer to what portsmgr / whoever thinks is the best approach. Using cp -r is convenient for having check-plist warn about missing / stable plist entries in the future as the port gets updated.

graphics/drm-current-kmod/pkg-message
8 ↗(On Diff #59901)

Ok. We can eventually use pkg-message.in if needed for the other ports in the future. I agree with focusing on just this port for now though. virtualbox-ose-kmod is another port that probably should be updated to use this once we are happy with the framework.

Having to specify each source and header file seems like a huge maintenance burden. For every update in Linux version files get added, removed and renamed. Some sort of recursive directory install would be nice...

Having to specify each source and header file seems like a huge maintenance burden. For every update in Linux version files get added, removed and renamed. Some sort of recursive directory install would be nice...

This is just how ports work, the package list (pkg-plist) must contain all files that are to be installed, otherwise pkg doesn't know how to create a package from the files in the staging area.

In D20990#455805, @jhb wrote:

You have added all the sources to the pkg-plist, but you don't always install all the sources, it's dependent on the architecture. This works on amd64, since all sources are used there, but not on other platforms.

Hmm, I thought I matched what I do. I always install 'drivers' and 'include' which contains the bulk of the sources. The only conditional bits are the subdirectories that contain the module Makefiles, and those
are conditional (e.g. i915/Makefile)

You are right, I just didn't see it the first time, sorry for that. You probably have to give the same treatment to amdgpu and amdkfd as you give to i915 and vmgfx well though, since they aren't built on i386, and other architectures.
It feels a bit like we need to make a decision, either we install all the sources, including makefiles, and just add safeguards in them to not build everything everywhere, or perhaps we should avoid extracting all sources, for instance i915 sources on ppc64. I don't know if this is worth the effort though.

jhb added a comment.Jul 31 2019, 11:03 PM

Having to specify each source and header file seems like a huge maintenance burden. For every update in Linux version files get added, removed and renamed. Some sort of recursive directory install would be nice...

This is just how ports work, the package list (pkg-plist) must contain all files that are to be installed, otherwise pkg doesn't know how to create a package from the files in the staging area.

There are some ways to do automatic plist general, but my understanding is that portmgr@ generally frowns on it as they want to preserve the ability to grep for a file in all the pkg-plist files. *shrug* FWIW, I generated the list of files with 'find' fairly easily. 'make check-plist' is also handy to let you know if you have it wrong.

In D20990#455805, @jhb wrote:

You have added all the sources to the pkg-plist, but you don't always install all the sources, it's dependent on the architecture. This works on amd64, since all sources are used there, but not on other platforms.

Hmm, I thought I matched what I do. I always install 'drivers' and 'include' which contains the bulk of the sources. The only conditional bits are the subdirectories that contain the module Makefiles, and those
are conditional (e.g. i915/Makefile)

You are right, I just didn't see it the first time, sorry for that. You probably have to give the same treatment to amdgpu and amdkfd as you give to i915 and vmgfx well though, since they aren't built on i386, and other architectures.
It feels a bit like we need to make a decision, either we install all the sources, including makefiles, and just add safeguards in them to not build everything everywhere, or perhaps we should avoid extracting all sources, for instance i915 sources on ppc64. I don't know if this is worth the effort though.

I think the top-level Makefile already DTRT and only descends into relevant subdirectories. I think trying to split up the raw sources would be a big PITA and that excluding a handful of Makefiles is probably not worth it, so would be fine with just installing all of the Makefiles and sources always.

In D20990#458950, @jhb wrote:

Having to specify each source and header file seems like a huge maintenance burden. For every update in Linux version files get added, removed and renamed. Some sort of recursive directory install would be nice...

This is just how ports work, the package list (pkg-plist) must contain all files that are to be installed, otherwise pkg doesn't know how to create a package from the files in the staging area.

There are some ways to do automatic plist general, but my understanding is that portmgr@ generally frowns on it as they want to preserve the ability to grep for a file in all the pkg-plist files. *shrug* FWIW, I generated the list of files with 'find' fairly easily. 'make check-plist' is also handy to let you know if you have it wrong.

Agreed. From a ports perspective, this is usually not very hard to get right, and the tools we have (poudriere, check-plist, ...) are quite good at helping out.

In D20990#455805, @jhb wrote:

You have added all the sources to the pkg-plist, but you don't always install all the sources, it's dependent on the architecture. This works on amd64, since all sources are used there, but not on other platforms.

Hmm, I thought I matched what I do. I always install 'drivers' and 'include' which contains the bulk of the sources. The only conditional bits are the subdirectories that contain the module Makefiles, and those
are conditional (e.g. i915/Makefile)

You are right, I just didn't see it the first time, sorry for that. You probably have to give the same treatment to amdgpu and amdkfd as you give to i915 and vmgfx well though, since they aren't built on i386, and other architectures.
It feels a bit like we need to make a decision, either we install all the sources, including makefiles, and just add safeguards in them to not build everything everywhere, or perhaps we should avoid extracting all sources, for instance i915 sources on ppc64. I don't know if this is worth the effort though.

I think the top-level Makefile already DTRT and only descends into relevant subdirectories. I think trying to split up the raw sources would be a big PITA and that excluding a handful of Makefiles is probably not worth it, so would be fine with just installing all of the Makefiles and sources always.

I'm OK with this. Perhaps then i915/Makefile and vmgfx/Makefile should be installed in all cases, mostly for completeness and/or symmetry or something. :)

manu added a subscriber: manu.Aug 1 2019, 12:49 PM
manu added inline comments.
graphics/drm-current-kmod/Makefile
82 ↗(On Diff #59901)

You can use COPYTREE_SHARE

jhb updated this revision to Diff 60375.Aug 1 2019, 8:10 PM
  • Install all sources always.
  • Use COPYTREE_SHARE and INSTALL_DATA instead of CP.
jhb marked 3 inline comments as done.Aug 6 2019, 6:58 PM
manu accepted this revision.Aug 6 2019, 7:06 PM
This revision is now accepted and ready to land.Aug 6 2019, 7:06 PM
jhb updated this revision to Diff 60520.Aug 6 2019, 10:22 PM
  • Add a SOURCE option to control installation of sources.
This revision now requires review to proceed.Aug 6 2019, 10:22 PM
zeising added inline comments.Aug 6 2019, 10:39 PM
graphics/drm-current-kmod/Makefile
77–79 ↗(On Diff #60520)

Is this needed in all cases, or just the SOURCE-on case?

81–87 ↗(On Diff #60520)

This can be expressed as post-install-SOURCE-on, instead of the .if .endif. Like this.

post-install-SOURCE-on:
        ${MKDIR} ${KMODSRC}
        ...
jhb updated this revision to Diff 60522.Aug 6 2019, 11:10 PM
jhb marked an inline comment as done.
  • Use post-install-SOURCE-on
jhb added inline comments.Aug 6 2019, 11:19 PM
graphics/drm-current-kmod/Makefile
77–79 ↗(On Diff #60520)

It doesn't hurt to always create an obj dir, and if we turned this into a Uses/kmod.mk thing where we say 'USES= kmod=obj', I suspect we would just leave it always on as it wouldn't be worth the extra .if's to make it optional. I could make it optional if you wanted, though the MAKE_ENV above also has to be optional (and at that point you might want to wrap it in a single .if vs using pre-build-SOURCE-on)

zeising accepted this revision.Aug 7 2019, 12:53 AM
This revision is now accepted and ready to land.Aug 7 2019, 12:53 AM
jhb updated this revision to Diff 60527.Aug 7 2019, 1:21 AM
  • Bump PORTREVISION.
This revision now requires review to proceed.Aug 7 2019, 1:21 AM
zeising accepted this revision.Aug 7 2019, 1:24 AM
This revision is now accepted and ready to land.Aug 7 2019, 1:24 AM
This revision was automatically updated to reflect the committed changes.