Page MenuHomeFreeBSD

xen/devices: purge uses of intr_machdep.h
ClosedPublic

Authored by ehem_freebsd_m5p.com on Apr 23 2021, 10:43 PM.

Details

Summary

Devices in sys/dev should be architecture-independent and NOT #include
intr_machdep.h.

Diff Detail

Repository
R10 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

I was looking at history and trying to figure out how things should look when submitted. Finally figured out, rather than introducing a near-empty sys/arm64/include/intr_machdep.h file, what is actually needed is to get rid of references to machine/intr_machdep.h. Now an issue of purging references to it, while ensuring I don't break anything (erk!).

So, these files still contain references to definitions provided by intr_machdep.h? Do you know which ones/how many?

Since test build (arm64, amd64 and i386) succeeded, I believe the #include of the header was the last remaining reference.

sys/dev/xen/control/control.c is rather messier (D29306 and D29599).

Any chance of review for D29959?

Yes, making these 3 files properly architecture-independent is as simple as removing the #include of this header. This might have been needed in the past (most likely due to earlier version of the event channel code), but now the #include is the last architecture-dependent bit.

This is not merely cleanup, there is genuine functional change. This header doesn't exist on other architectures (notably arm64) and so removing it allows these to build (and function) on other architectures.

This seems fine, these drivers use the xen_intr_ interfaces rather than anything contained in the header. Just one question about pcifront.c.

sys/dev/xen/pcifront/pcifront.c
55 ↗(On Diff #88067)

Hmm it seems like this file is not hooked up to the build. @royger should it be?

This revision is now accepted and ready to land.Tue, Oct 12, 1:19 AM

LGTM, will push

sys/dev/xen/pcifront/pcifront.c
55 ↗(On Diff #88067)

Not really, this was part of a very old Xen import but pcifront is only used for PV and FreeBSD doesn't support that mode.This code has never been built and should be purged. I can remove it when pushing this patch.

sys/dev/xen/pcifront/pcifront.c
55 ↗(On Diff #88067)

Cool, removing code is always the best result :)

sys/dev/xen/pcifront/pcifront.c
55 ↗(On Diff #88067)

As long as functionality is maintained. I note some workloads offer better performance in PV mode. PV mode though requires significant changes closer to the core. PV mode is still supported by Linux, but that may get more funding from Citrix.

sys/dev/xen/pcifront/pcifront.c
55 ↗(On Diff #88067)

FWIW it's mostly SuSE the one that currently has the maintainership burden of classic PV on Linux (and Xen support for Linux in general).

This revision was automatically updated to reflect the committed changes.