linuxkpi: Restore the KBI for struct pci_driver

[imp]

Summary

The 13.0 version of struct pci_driver was 92 or 184 bytes on 32 or 64
bit systems respectively. We recently added bsd_probe_return at the end
of this struct, breaking the KBI.

Fix this by remiving isdrm. We don't need it because we can do a strcmp
in the few places that need it as they aren't performance critical. In
its place move the newly added bsd_probe_return to that slot. It's the
same size in all our supported KBIs due to padding, etc.

Also add 32 or 64 bytes of padding to this structure in the _spare field
like we should have done when we branched stable/13, but neglected to do
so since we didn't properly anticipate the need.

These spare fields are unsafe to use before 13.1 is the oldest supported
13.x release since drivers compiled on 13.0 won't have that space
reserved and we'll step on something else using them. This isn't 100%
KBI compatible through the 13.x release branch, but is compatible enough
so that drm packages build on the oldest supported release will work on
the latest stable/13 and any newer releases. It's not ideal, but makes
the best of a bad situation and is a pragmatic approach that belatedly
builds in some future proofing.

Direct commit to stable/13 because this is not relevant to main.

Sponsored by: Netflix

[imp]

fix commit message

[imp]

strcmp returns 0 when equal. <doh>

[bz]

Can this be two independent changes please?

The spares shouldn't go onto just this struct likely and have nothing to do with the fix for the 13.0-built drm binary modules on 13.1-R. The spares should probably also come from main and not directly from stable/13?

[bz]

Add x11 to subscribers so they know how things proceed.

[imp]

I can do this as two commits... but the spare doesn't need to go into main first... There's no need... While we usually go through head to stable, it's just before or just after a stable branch is made. Now that stable/13 has been around for a while, there's little to no need since we don't need to (and likely can't possibly) run 13.0 built binaries on main/14.0 CURRENT.

I could run the other change through main, but I don't see the point: we don't really need it there, this does provide a minor optimization, etc. I plan on making these two things as direct commits to stable/13. First the change and assert (with the old size) and then then spare + update size. I'd then merge both to releng/13.1 since we need both there (1 to fix a bug and 1 to future proof this code on stable/13, otherwise we're waiting til 13.1 goes unsupported and that's quite a while from now).

[imp]

I'm happy to put, btw, spares in other structures as well, but I'm not familiar enough with the code to know good candidates. I can also to the Static_assert for those as well... Got any suggestions?

[bz]

In D34754#787677, @imp wrote:

I'm happy to put, btw, spares in other structures as well, but I'm not familiar enough with the code to know good candidates. I can also to the Static_assert for those as well... Got any suggestions?

See 0437d10e359ea1cbefff8d17cd18ca491dbbd5d7 for header files, and I'd also suggest in linux/device.h.

Networking and wireless will one day also have the same fate but currently in-tree wireless seems to be the only consumer and drm-kmod is not an overlap.

[bz]

In D34754#787676, @imp wrote:

I could run the other change through main, but I don't see the point: we don't really need it there,

If spares are not in main stable/14 won't have spares either and history will repeat itself. In main spares are usually not touched ; unless they have been feature specific spares and get removed in main as the feature got implemented. We used to plan then even before .0 releases in the old days: https://wiki.freebsd.org/Releng/9.0TODO/Spares in case people know they need extra.

[imp]

In D34754#787754, @bz wrote:

In D34754#787677, @imp wrote:

I'm happy to put, btw, spares in other structures as well, but I'm not familiar enough with the code to know good candidates. I can also to the Static_assert for those as well... Got any suggestions?

See 0437d10e359ea1cbefff8d17cd18ca491dbbd5d7 for header files, and I'd also suggest in linux/device.h.

All the structures in that commit are allocated by the module and so the size isn't encoded into the driver's module. only offsets are.

I really don't have the time to do a deep dive into each and every structure to see which ones the drivers allocate and thus need spares.

I see struct dev_pm_ops as being allocated inside the driver (but It's unlikely to grow additional fields we'll use). I'm not sure I see the point of padding this. On the flip side, it isn't hard to do.

struct device is allocated in device_create_groups_vargs which is a static inline in a .h file (I really hate with a passion the extreme over-inlining in linuxkpi: it makes the KBI so much harder to achieve).
Ditto for struct class in class_create. These two are easy to do.

I can't find any others that might be used, though, but I'm likely missing something...

Networking and wireless will one day also have the same fate but currently in-tree wireless seems to be the only consumer and drm-kmod is not an overlap.

The plan is to bring drm-kmod back into the base in 14. :)

[imp]

In D34754#787755, @bz wrote:

In D34754#787676, @imp wrote:

I could run the other change through main, but I don't see the point: we don't really need it there,

If spares are not in main stable/14 won't have spares either and history will repeat itself. In main spares are usually not touched ; unless they have been feature specific spares and get removed in main as the feature got implemented. We used to plan then even before .0 releases in the old days: https://wiki.freebsd.org/Releng/9.0TODO/Spares in case people know they need extra.

Yea, Putting them in now just makes the structures bigger w/o much good effect. I'm not sure I'm sold on it as a good idea, but there's at least a fair amount of this in the network code. I'd like other's opinions who work more on linuxkpi as well. The spares should have a kassert with the size, otherwise that won't be in stable/14 either.... Though in 14 with all the main consumers of this moved back into the tree, I wonder if the cost/benefit is too low to bother...

[bz]

I can confirm that an amd64 releng/13.1 kernel with this does load the i915kms and drm and I get the drmn0 line as well after a short flicker on the screen; I have no X on external USB drive to further test it but the "silent error" seems gone.

[bz]

Ignoring all the other things going on now .. do you plan to commit this w/ or w/o the spares to stable/13 within the next 24 hours?

[imp]

In D34754#788157, @bz wrote:

Ignoring all the other things going on now .. do you plan to commit this w/ or w/o the spares to stable/13 within the next 24 hours?

I do. It's a known issue that's resolved by this, even if it could be done better if we had more time to properly study things...
There's some other commits that make things better, but I'm not sure if I'll get them into stable/13 in time for them to be in 13.1.