Page MenuHomeFreeBSD

xen/xen-os: move inclusion of machine/xen-os.h later
AcceptedPublic

Authored by ehem_freebsd_m5p.com on Apr 17 2021, 5:42 PM.

Details

Summary

Several of x86 enable/disable functions depend upon the xen*domain()
functions. As such the xen*domain() functions need to be declared
before machine/xen-os.h.

Diff Detail

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

Event Timeline

I had been thinking @royger was going to be checking for x86, but since latest needed checking, I tried and found this was an issue. Seems rather a number of kernel headers fail to include headers they depend upon as well.

This came about due to a WiP which needed a bit of checking on x86. Then found there hadn't been much checking to ensure things didn't get broken on x86.

This though brings up an issue: What is the FreeBSD stance on headers which require other headers?

For instance <sys/bus.h> requires multiple other headers to be #include'd, otherwise you get build errors. Notably uint64_t is needed, yet <sys/bus.h> does not #include any definition itself.

This came about due to a WiP which needed a bit of checking on x86. Then found there hadn't been much checking to ensure things didn't get broken on x86.

This though brings up an issue: What is the FreeBSD stance on headers which require other headers?

For instance <sys/bus.h> requires multiple other headers to be #include'd, otherwise you get build errors. Notably uint64_t is needed, yet <sys/bus.h> does not #include any definition itself.

Generally, users of the headers are expected to include the right prerequisite headers. sys/types.h is rarely included which is how uint64_t comes in. This is done to avoid quadratic expansion of the headers one needs as recursion happens.

However, there's several exceptions to this rule when the definitions are in _foo.h files that are shared between foo.h and other headers.

D29811 is a notable concern. I discovered it had been quite some time since I had last tried building x86 and upon trying I discovered things had gotten broken. D29811 is marked as the parent for several since I believe those need D29811 first.

Fix a lurking gotcha. sys/x86/xen/xen_intr.c had been directly including
machine/xen/xen-os.h, which it shouldn't have been doing. This is closely
related so I think it is okay to include with this commit.

D29811 could use some attention. It is marked as parent for several since those break on x86 without D29811 (wrong order of function declaration, several call xen_*_domain()).

Hmm, updated this locally and hadn't gotten it uploaded. Simply placing a #error in x86/xen/xen-os.h to prevent direct inclusion.

Seems fine, given the child revisions. Perhaps these changes can all be squashed for the final commit?

This revision is now accepted and ready to land.May 14 2021, 1:42 PM