Pushed it here: https://github.com/amshafer/freebsd/tree/this_module

Added parentheses

Yes, thanks. I don't have a committer bit.

Thanks, updated to just remove the module_get() bits for now.

Updated with jhb's proposal. Also implemented module_get and module_put by casting to linker_file_t and bumping the refcount. Retested with PRIME offloading to confirm this still fixes the panic. Unlike the previous (similar) version this handles the statically compiled module case.

Any update on getting this merged?

I believe that would work and I agree it would be much simpler. @kib is there something the (b) variant provides that (a) would be missing if we went with @jhb's proposal? It seems like they provide the same functionality?

I guess the last question is what linuxkpi macro should actually create the global? I had previously used LKPI_DRIVER_MODULE, but not everywhere uses that. I guess we could make our own DEFINE_THIS_MODULE() macro and then call it from LKPI_DRIVER_MODULE, and add calls elsewhere as needed?

It seems the way it works is that THIS_MODULE is a NULL pointer during "builtin" (statically building modules into the kernel), and points to the struct module of the dynamically loaded module otherwise. I think we could do something similar where we get the current linuxkpi behavior (THIS_MODULE is null) if we aren't building a dynamically loaded module?

how would any of proposals handle LKPI modules statically compiled into the kernel?

Instead, it should be only resolved locally, causing undefined symbol error if attempt is made to reference it from kld without the module symbol (section ?).

So what is the expected use of THIS_MODULE in Linux? Determine that current module is not that module, or something else?

Updated with KLD_DPF debug statement.

Updated to be generic. I changed the name to __this_linker_file to better represent
what the variable points at. On linux __this_module points at a struct module, so
for us we point at a linker_file_t.

Updated with style, thanks.

There isn't a PR, this has been a longstanding issue with nvidia-drm that I'm
finally getting around to solving.

Thanks for all the suggestions! I've implemented it by doing the special
casing in linker_file_lookup_symbol_internal as suggested. It looks like
this does work, I can see different owner fields populated in nvidia-drm
and drm. Also confirmed multiple usages of THIS_MODULE in drm.ko point to
the same thing.

It sounds like linux has insmod set up the __this_module variable when a module is loaded. If you're asking about doing something equivalent then I don't have a good estimate for how hard that would be. I don't know the details though.

That's a promising idea. I wonder if instead of having an unusual inline function we could instead make it part of the build setup. Something like setting KBUILD_MODNAME_HASH to whatever sha1 -s "$KBUILD_MODNAME" returns as part of the Makefile? That seems like it might be a little cleaner and easier to debug. Only downside is we have to update all of the makefiles that use KBUILD_MODNAME, I can't think of a way to automate it off the top of my head.

Bump __FreeBSD_version for drm-kmod

Since there were no comments I've gone ahead with the strategy I
outlined in my last comment: Have drm-kmod create a global variable
during LKPI_DRIVER_MODULE and point THIS_MODULE at it.

Maybe an interesting solution would be to do something a bit more DRM specific using LKPI_DRIVER_MODULE. Afaict only the DRM drivers will run into the issue that motivated this change. There's no reason we couldn't do something like this in base or just move the bits I mentioned into base.

Yes sorry, bumped PORTREVISION

This has been tested by @grahamperrin

Moved BUILD_DEPENDS to before as suggested.

Updated to handle 13.2 by patching conftest.sh

Updated to add back in RUN_DEPENDS

In D44305#1010541, @arrowd wrote:

On FreeBSD 13 this results in
../common/inc/nv-mm.h:110:31: error: too many arguments to function call, expected 5, have 8

That's for struct class though, right? I'm talking about struct module, and struct file_operations. Here's an example usage from nvidia-drm: https://github.com/amshafer/nvidia-driver/blob/535.98/nvidia/src/nvidia-drm/nvidia-drm-gem.c#L156. Iirc there's a couple similar ones in other drivers in base.

I don't think so? The 550 build has been broken because of this so no actual version has been "released" for nvidia-drm-510-kmod with 550, so it doesn't seem like bumping the revision would do anything? I can if needed though

I'm open to other suggestions about how we can do this. We definitely want THIS_MODULE to be non-zero, but having it be an opaque pointer that isn't a struct module isn't that great. Barring fully implementing module in linuxkpi I think something like this could make sense to allow emulated code to check if devices/dmabufs/etc come from the same module?

I think you need to update to a more recent CURRENT, that was added at the end of december I think.

Odd I hadn't seen that warning. I removed the PORTVERSIONS and also reset PORTREVISION.

Update to add a pkg-message warning about the new modesetting sysctl

Rebase on latest main

Note that this shouldn't be merged until after PR 277028

Motivated by reports such as: https://github.com/amshafer/nvidia-driver/issues/21

It doesn't build, I got notified of this by a failing build: https://pkg-status.freebsd.org/beefy18/data/main-amd64-default/p384f6f077a9c_s1ff41cad71/logs/nvidia-drm-515-kmod-535.104.05_2.log

I know there's a lot of updates to the DRM infrastructure (terrific btw!) happening so feel free to ping me if there's anything you think might break nvidia-drm and you want me to test. I've verified 6.1-lts builds with nvidia-drm fine and I plan on testing that on my machines at some point soon.

This should fix recent package build failures. Tested building locally.

Off the top of my head I thought I tested with GENERIC but because that KASSERT didn't fire I'm assuming I had GENERIC-NODEBUG installed, which I normally do on half my machines.

haha that certainly looks a bit more complicated :)

Using PKGNAME lets me include the PORTREVISION in addition to the previous fields. That way it will look for tags such as nvidia-drm-515-kmod-535.98_1 instead of nvidia-drm-515-kmod-535.98. This seemed like the easiest way to do this.

Fixed styling

This is part of a fix for https://github.com/amshafer/nvidia-driver/issues/20

The code is still NVIDIA code, I've just modified it for compatability. The nvidia-drm-kmod port's Makefile is mine though, is that what you're referring to? I think the license of nvidia-drm-kmod should match the child ports to avoid anyone getting confused and thinking nvidia-drm.ko is BSD licensed.

Setting LICENSE_FILE= ${WRKSRC}/nvidia/doc/license.txt in Makefile.common should be fine for the child ports, but for the metaport I'm not sure the best way to include that? For nvidia-drm-kmod should I just copy that license file in the directory and reference that in LICENSE_FILE?

It's documented here: https://github.com/amshafer/nvidia-driver/blob/535.86.05/UPDATING.md

One thing of interest to reviewers here: how should we (meaning me) best coordinate matching updates of nvidia-drm-kmod with updates to nvidia-driver? nvidia-drm-kmod isn't as simple as bumping a version number since I have to populate a driver branch in github for a particular version.

Fixed 515 to actually be 515, reworked RUN_DEPENDS, I think I got the pre/post.mk stuff you recommended correct but let me know if not.

I originally added NVIDIA_DISTVERSION since we will need to have multiple nvidia-drm-kmod ports which match the different versions of drm-kmod. For now basically nvidia-drm-510-kmod and nvidia-drm-515-kmod. It seemed easiest to have one Makefile.version in nvidia-driver which defines the version number to avoid having to update it by hand in all the ports that depend on nvidia-drm, instead you'd only have to update the distinfos. I added something similar to drm-kmod a bit ago. My goal was make one common nvidia-drm-kmod Makefile.common which uses NVIDIA_DISTVERSION, NVIDIA_GH_TAGNAME, and DRM_KMOD_GH_TAGNAME to select the versions of everything, and each nvidia-drm-*-kmod port imports the Makefile.versions it wants to reference.

Bump version to 535.86.05

Updating this review to only add Makefile.version instead of updating the
nvidia-driver version now that it is at 535.54.03.

Updated to 9.12, switched to https, fixed a warning about requiring
libp11-kit that make stage-qa threw.

Fixed linux-nvidia-libs distinfo

PR: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=271523

Adding missing linux-nvidia-libs bits

Here's the rundown of my testing: on my three laptops things look to be working as I expected. I don't replicate anything like what you're seeing on the single-GPU system but I'll get to that later and maybe we can figure out what your issue is. Here's what I see:

• On my single-GPU Intel laptop it starts properly with no xorg.conf or 20-intel.conf.
• On my Intel+NVIDIA PRIME laptop it works with no xorg.conf.
  • 20-intel.conf is not required if nvidia-drm.ko is loaded, the 20-nvidia-drm-outputclass.conf from the blog post is however.
  • If nvidia-drm.ko is not loaded, then the X server sees two devices and isn't sure which one to use, so it bails and tells you to specify frambuffer busIDs. This is where 20-intel.conf will fix things, as it tells X which one to use. I'll have to check out on Linux if this is the same behavior. I wouldn't be surprised if it is since they usually expect nvidia-drm to be present therefore avoiding the issue, but we'll see.
• On my Intel+NVIDIA PRIME laptop your skeleton xorg.conf from your last comment works fine.
• On my AMD+NVIDIA PRIME laptop it works with no xorg.conf, configs from the blog post.

Or for the single-GPU system a normal xorg.conf

I think it's because you're missing the files in xorg.conf.d? You need something like this:

Abandoning since this was included in D39886

Updated to merge in D39887 and bump PORTREVISION

Closing this since it was already submitted in 4085bde9fa2e

Here's the github PR handling this for older versions before we moved this function into base: https://github.com/freebsd/drm-kmod/pull/218