linuxkpi: MODULE_DEVICE_TABLE to MODULE_PNP_INFO
ClosedPublic
Actions

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

Details

Reviewers

imp
kib
• hselasky
manu

Group Reviewers

x11

Commits

rG7380d82b53d6: LinuxKPI: pci.h MODULE_DEVICE_TABLE to MODULE_PNP_INFO
rGa94b233d5d71: LinuxKPI: pci.h MODULE_DEVICE_TABLE to MODULE_PNP_INFO
rG5493c6277a26: LinuxKPI: pci.h MODULE_DEVICE_TABLE to MODULE_PNP_INFO

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

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 33966
Build 31159: arc lint + arc unit

Event Timeline

bz created this revision.Oct 3 2020, 6:12 PM

Herald added a subscriber: emaste. · View Herald TranscriptOct 3 2020, 6:12 PM

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

Harbormaster completed remote builds in B33966: Diff 77819.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:
..

• hselasky added inline comments.Oct 3 2020, 6:34 PM

sys/compat/linuxkpi/common/include/linux/pci.h
73	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.

In D26651#593796, @hselasky wrote:

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
87	Why not include the subvender and subdevice? There's no reason to omit them.

In D26651#593797, @hselasky wrote:
@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.

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.

Harbormaster completed remote builds in B33974: Diff 77842.Oct 4 2020, 2:36 PM

bz marked an inline comment as done.Oct 4 2020, 2:36 PM

• hselasky accepted this revision.Oct 4 2020, 3:01 PM

This revision is now accepted and ready to land.Oct 4 2020, 3:01 PM

imp accepted this revision.Oct 4 2020, 3:53 PM

bz added a subscriber: zeising.Oct 5 2020, 5:49 PM

lattera-gmail.com added a subscriber: lattera-gmail.com.Dec 29 2021, 5:45 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.