Page MenuHomeFreeBSD

linuxkpi: MODULE_DEVICE_TABLE to MODULE_PNP_INFO
ClosedPublic

Authored by bz on Oct 3 2020, 6:12 PM.

Details

Summary

Implement (hack) up MODULE_PNP_INFO() support in the linuxkpi
for the Linux MODULE_DEVICE_TABLE.
Sadly we need to ensure there is a DRIVER_MODULE() (or probably just
a DECLARE_MODULE but that makes not much difference) before it
(also see man page).

Also sadly add subvendor, etc. fields as comments at the end
to the descriptor_string seem to prevent autoloading and that
should be sorted out separately. To kldxref or devinfo the
devices currently will simply look like duplicates.

Sponsored by:  The FreeBSD Foundation

Diff Detail

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

Event Timeline

bz requested review of this revision.Oct 3 2020, 6:12 PM

I'd still love to update this so that we can have the full set but I need to find the bug first that prevents this (with the comments) from working. You can still see how this turns out (just with less columns):

Module lkpi_iwl_hw_card_ids_pci in if_iwl.ko
Version: if iwl.1 kmod if_iwl.ko
PNP info for bus pci format I:vendor;I:device;I:#;I:#;I:#;I:#; 263 entries (if_iwl.ko)
   0x8086:0x8b1:0xffffffff:0x4070:0:0:
   0x8086:0x8b1:0xffffffff:0x4072:0:0:
   0x8086:0x8b1:0xffffffff:0x4170:0:0:
   0x8086:0x8b1:0xffffffff:0x4c60:0:0:
   0x8086:0x8b1:0xffffffff:0x4c70:0:0:
   0x8086:0x8b1:0xffffffff:0x4060:0:0:
   0x8086:0x8b1:0xffffffff:0x406a:0:0:
   0x8086:0x8b1:0xffffffff:0x4160:0:0:
   0x8086:0x8b1:0xffffffff:0x4062:0:0:
   0x8086:0x8b1:0xffffffff:0x4162:0:0:
   0x8086:0x8b2:0xffffffff:0x4270:0:0:
   0x8086:0x8b2:0xffffffff:0x4272:0:0:
   0x8086:0x8b2:0xffffffff:0x4260:0:0:
   0x8086:0x8b2:0xffffffff:0x426a:0:0:
   0x8086:0x8b2:0xffffffff:0x4262:0:0:
   0x8086:0x8b1:0xffffffff:0x4470:0:0:
   0x8086:0x8b1:0xffffffff:0x4472:0:0:
..
sys/compat/linuxkpi/common/include/linux/pci.h
76

DEVMETHOD_END is the right for { 0, 0 }

Change looks good in total.

Also sadly add subvendor, etc. fields as comments at the end

to the descriptor_string seem to prevent autoloading and that

Linux uses -1U for wildcard. Maybe devmatch needs support for that too!

@bz

I:vendor;I:device;I:#;I:#;I:#;I:#

Format should use "J" instead, I guess, because it allows -1 as ignore value.

Also sadly add subvendor, etc. fields as comments at the end

to the descriptor_string seem to prevent autoloading and that

Linux uses -1U for wildcard. Maybe devmatch needs support for that too!

We do support that...

sys/compat/linuxkpi/common/include/linux/pci.h
90

Why not include the subvender and subdevice? There's no reason to omit them.

@bz

I:vendor;I:device;I:#;I:#;I:#;I:#

Format should use "J" instead, I guess, because it allows -1 as ignore value.

-1 is the don't care value.

bz marked an inline comment as done.

Use DEVMETHOD_END.
Include subvendor/subdevice as V32 to deal with the -1 wildcard.
Add the bus name to the name filed in the driver_t.

bz marked an inline comment as done.Oct 4 2020, 2:36 PM
This revision is now accepted and ready to land.Oct 4 2020, 3:01 PM

Adding x11 to reviewers as it will also mean that drm modules will start autoloading and no longer need the kld_list entry in rc.conf or other.
while I had tested this a long time ago I would be happy if someone from the drm team would confirm that it is good to go in.

We have a x11 meeting tomorrow, as luck would have it, and I'll ask about it then.

In D26651#764514, @bz wrote:

Adding x11 to reviewers as it will also mean that drm modules will start autoloading and no longer need the kld_list entry in rc.conf or other.
while I had tested this a long time ago I would be happy if someone from the drm team would confirm that it is good to go in.

As long as the blacklist from devmatch works autoloading wouldn't be worse that what most people do which is using kld_list.
I only want the blacklist to work for dev as there is still some problems with unloading drm drivers.

devmatch_blocklist="if_iwlwifi" works; I'll try to see if I can check with drm-kmod on the native host tomorrow as well.

I just found that you have already duplicated parts of this in linuxkpi/bsd/include/linux/module.h in drm-kmod; seems that only generate entries for i915kms.ko for me. That means the functionality is there already and devmatch_blocklist="" would have to work already. I also get entries for radeonkms and amdgpu now as it seems and of course i915kms.

Funny enough i915kms.ko did not auto-load; I assume that is because vgapci0 is attached, and with that we then no longer devmatch load things. So the entire thing seems a non-started for drm-kmod anyway.

Unless someone enlightens me how auto-loading on drm-kmod drivers should work or vetos on this the next 24 hours I'll get it in.

In D26651#764929, @bz wrote:

I just found that you have already duplicated parts of this in linuxkpi/bsd/include/linux/module.h in drm-kmod; seems that only generate entries for i915kms.ko for me.

Mhm right, we should probably remove this.
We also have MODULE_DEVICE_TABLE for all drivers present.

That means the functionality is there already and devmatch_blocklist="" would have to work already. I also get entries for radeonkms and amdgpu now as it seems and of course i915kms.

Funny enough i915kms.ko did not auto-load; I assume that is because vgapci0 is attached, and with that we then no longer devmatch load things. So the entire thing seems a non-started for drm-kmod anyway.

Weird.

Unless someone enlightens me how auto-loading on drm-kmod drivers should work or vetos on this the next 24 hours I'll get it in.

Yeah go ahead, we'll deal with this later.