xen/intr: move x86 bits into xen_arch_isrc_t structure

[ehem_freebsd_m5p.com]

Summary

This moves quite a few of the x86-specific variables into a structure
separate from struct xenisrc, a first step in breaking them off.

Ideally these would never have left sys/x86/xen, but breaking them off at
the same time as moving the main portion of the file would have been
ugly.

The original version of this commit moved xi_activehi and xi_edgetrigger
into xen_arch_isrc, but their usages disappeared with
ac3ede5371af34b0a89fa72b7a5bb706457b99ad. As a result purge them
instead.

Other architectures (ARM) assigns interrupt vectors via other means, thus
having the vector number in the general structure makes no sense.

This was inspired by the work of Julien Grall <[e-mail]>,
2015-10-20 09:14:56, but now only preserves the "xen_arch_intr_t" and
the addition of arch-intr.h.

[royger]

sys/xen/xen_intr.c

119

Should this be added to an x86 header instead of here so that you don't have to move it afterwards?

I expect you will end up moving it in a later commit?

121

Make it unsigned int while at it.

127

While there could you add a CTASSERT that &xenisrc == &xenisrc->xi_arch.xai_intsrc where the code is relying on that property?

I think you can use offsetof to check the field is at offset 0 at build time.

129

Can you place the removal of those fields into a separate patch and make it part of the branch to be committed?

[ehem_freebsd_m5p.com]

Update to newer version. Definitely resolve 2 comments, other two I'm split on.

[ehem_freebsd_m5p.com]

Trying to keep this commit distinct from the one where @julien_xen.org moved several pieces of the file back to sys/x86... Still using those names even though this represents a distinct rework of some of the pieces. There are a fair number of trivial wrappers around headers present, I prefer symbolic links since git does support those.

I've got a preliminary version of the CTASSERT in place, appeared at to function for the purpose you were thinking of. I note xen_intr.c already makes the assumption in many places. Notably nearly every function which takes a struct intsrc *base_isrc as an argument implicitly assumes the struct intsrc * can be converted to a struct xenisrc *. I agree this is a Good Idea to add them.

You're really that interested in having the removal of 2 lines be a distinct commit? I only noticed the situation when I looked at the diff then noticed xai_activehi and xai_edgetrigger no longer occurred anywhere. Until that point those were in struct xen_arch_isrc.

[ehem_freebsd_m5p.com]

I believe the CTASSERTS() added in D30909 should fulfill your desire for those. I agree they do make rather a bit of sense.

[ehem_freebsd_m5p.com]

Turning the behind the scenes into two commits. Was this the desired change?

[ehem_freebsd_m5p.com]

Getting the CTASSERT()s into D30648. Hopefully this is more satisfactory to reviewers, even though the final result is still the same in D30909.

[ehem_freebsd_m5p.com]

Switching back to the original approach. While the extra two disappear due to PVHv1 removal, they should still follow to x86 at this delta.

[ehem_freebsd_m5p.com]

Now notable D32876 shows how isrc->xi_cookie can move to isrc->xi_arch.xai_cookie. Just an issue of getting that decision made.

[ehem_freebsd_m5p.com]

Updating for present tree status. Keeping Phabricator up to date is difficult due to the number of commits needed for this project...