Page MenuHomeFreeBSD

Fix 13.x KBI for linuxkpi
AcceptedPublic

Authored by imp on Jul 31 2021, 6:11 AM.

Details

Reviewers
hselasky
bz
wulf
Group Reviewers
x11
Summary

direct: fix KBI for pci_dev

Move all the new elemenets to the end of the structure for 13. We
allocate this inside the linuxkpi code, so the size isn't enccoded in
client modules. However, the offsets to the different fields are
encoded. Tihs modifies 04456f711853, 40a215e38a4d, and 3a606aadf2e7
and will likely create merge conflicts there (and that's a good thing
since the elements need to be moved to the end of the structure when
merging).

direct commit: tweak irq_ent to be binary compatible

Since this is inlined into the clients, all clients have to agree on the
irq_ent offsets.

Restore visibility to linux_kmem_cache_free_rcu

linux_kmem_cache_free_rcu was made static in 10235ad0567f, however
client drivers depended on calling it directly. Make it visible again
to restore the 13.0-Release KBI for linuxkpi.

Bump FreeBSD_version to 1300513 for restoration of 13.0 KBI

Since the last few commmits to linuxkpi changes the KBI (this time back
to 13.0 release to restore the status quo of a couple weeks ago), you'll
need to recompile everything that uses it. The plus side is that our
packages (built using 13.0) for drm-kmod 5.4 work again on -stable
systems.

Sponsored by: Netflix

Test Plan

Wulf tested this and said it restored the ability to run 13.0-built drm-kmod on stable/13.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 40778
Build 37667: arc lint + arc unit

Event Timeline

imp requested review of this revision.Jul 31 2021, 6:11 AM
imp added reviewers: x11, hselasky, bz, wulf.

This is several commits that I've uploaded as one to make it easier to review.

sys/compat/linuxkpi/common/src/linux_slab.c
148–149

lkpi_kmem_cache_free_rcu function should be renamed back to linux_kmem_cache_free_rcu and made visible.
linux_kmem_cache_free_rcu_callback can be left intact. 10235ad0567f did not change it.

sys/compat/linuxkpi/common/include/linux/interrupt.h
56

Please lowercase this comment.

This revision is now accepted and ready to land.Jul 31 2021, 1:11 PM

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

--HPS

Has anyone tested that 13.0-RELEASE drm-kmod binary package drivers work again after this commit commit?
I'll go and have a look at some other things after lunch but will get back to you in a few hours.

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

I think if we want to garantuee things we need to start thinking a lot harder and write down "rules" as otherwise it'll not work.

If it was me, I'd say build a module for each release on the release and distribute that for 3rd party (external) code and whoever uses stable needs to track source and build modules herself. But with out ports/pkg system that doesn't work either so easily. Winning this is really hard given we still break in-kernel (non-compat) KPIs in stable branches regularly.

sys/compat/linuxkpi/common/include/linux/interrupt.h
56

I am not sure this will work as the allocation is in request_irq() in 13.0-R and that is an inline function from the header file.
That means the alloc in 13.0-R binary code will not allocate the full size but we might still (possibly) try to call thread_handler from the implementation file on a newer linuxkpi and that will fail accessing memory beyond the end of the struct unless no code from 13.0-R returns IRQ_WAKE_THREAD from the handler, which wasn't newly added. vmwgfx_irq.c from drm-kmod master seems to catch that case though itself, so we might be blessed enough.

I think you might be good to go with this @imp . I'd still love someone testing the 13.0-R package before the commit so we know it actually helps as if it doesn't this thing will keep hunting...

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

This causes a lot of grief for the drm-kmod folks, hence my attempt to make things compatible.

In D31363#706834, @bz wrote:

Has anyone tested that 13.0-RELEASE drm-kmod binary package drivers work again after this commit commit?

wulf@ tested this, which is why I made a run at getting it into the tree. We were compatible for 6 months until
the start of July.

In D31363#706863, @bz wrote:

I haven't cared so much a binary compatibility for now. People who want to use the LinuxKPI need to rebuild their code from sources.

I think if we want to garantuee things we need to start thinking a lot harder and write down "rules" as otherwise it'll not work.

Yes. We should, but we've been doing this in other parts of the kernel for a long time, and I just follow the same rules I follow there.

If it was me, I'd say build a module for each release on the release and distribute that for 3rd party (external) code and whoever uses stable needs to track source and build modules herself. But with out ports/pkg system that doesn't work either so easily. Winning this is really hard given we still break in-kernel (non-compat) KPIs in stable branches regularly.

Yes. That's also not a viable solution. We tried that through the 12.x release and it's been a disaster. We've kept other parts of the kernel with a stable ABI in the stable branches, so I thought I'd give this a shot. I even have a script to abstract all the .h files from the kernel so one can build a 13.1 version on a 13.0 (or 14.0) host, but that ran into some head winds so I set it aside before it was complete.

The real solution is to only distribute sources and force people that want to use this to compile everything to identically match their kernel, but that turns out to be a tricker thing to crack.

sys/compat/linuxkpi/common/include/linux/interrupt.h
56

Yea, I think that's why it is 'working' for wulf.
The linuxkpi heavy use of inlines do present problems in this area.
In fact, this is the crux of the issue for why this won't work with 5.5 based drm-kmod: it needs to use this functionality, but that doesn't exist on 13.0's sources (among other things, and I know I'm glossing over).

imp edited the test plan for this revision. (Show Details)

Tweaks, as requested

This revision now requires review to proceed.Jul 31 2021, 4:57 PM

I am fine with this.

This revision is now accepted and ready to land.Jul 31 2021, 6:58 PM

I tested it once again. After addition of missing prototype I am able to run XFCE on 13-STABLE with drm-fbsd13-kmod-5.4.92.g20210720_1 installed from pkg.FreeBSD.org packages.

sys/compat/linuxkpi/common/include/linux/slab.h
213

I have to place
void linux_kmem_cache_free_rcu(struct linux_kmem_cache *, void *);
line here to fix build.

/usr/src/sys/compat/linuxkpi/common/src/linux_slab.c:149:1: error: no previous prototype for function 'linux_kmem_cache_free_rcu' [-Werror,-Wmissing-prototypes]
linux_kmem_cache_free_rcu(struct linux_kmem_cache *c, void *m)
^
/usr/src/sys/compat/linuxkpi/common/src/linux_slab.c:148:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
void
^
static 
1 error generated.

make[1]: stopped in /usr/src

make: stopped in /usr/src