xen/intr: move interrupt allocation/release to architecture

[ehem_freebsd_m5p.com]

Summary

Inspired by the work of Julien Grall <[e-mail]>,
2015-10-20 09:14:56, but heavily adjusted.

Simply moving the interrupt allocation and release functions into files
which belong to the architecture. Since x86 interrupt handling is quite
distinct from other architectures, this is a crucial necessary step.

Identifying the border between x86 and architecture-independent is
actually quite tricky. Similarly, getting the prototypes for the
border right is also quite tricky.

Since interrupt structure allocation/release is now in architecture
code, the malloc structure needs to be passed. Some architectures also
need the device when allocating interrupts.

[ehem_freebsd_m5p.com]

Lots of action in D30936 as it is near the completion of the architecture-independent API for FreeBSD/Xen. I ended up splitting this off of D30909 due to the shear complexity involved in this portion. Unfortunately this has the effect of requiring xen_intr_pic being global/declared in sys/x86/include/xen/arch-intr.c between D30909 and D30936. Yet isolating this portion of the complexity seems worthwhile.

As you can see, not one, but 3 functions are fairly tightly interlinked and are best to move together. Similarly 3 static variables also need to stick together.

sys/xen/arch-intr.h

63–65 ↗

(On Diff #91464)

I'm not 100% certain I got these right. The device name is useful for the intr_create_event() call on arm64, though I don't believe that is absolutely required.

The bigger question is whether to have M_XENINTR remain in xen_intr.c, versus moving with xen_arch_intr_alloc() to the architecture-specific file. In earlier versions the call to malloc() had remained in xen_intr.c, but that became untenable when it was clear xen_intr_find_unused_isrc() should move to architecture (since identifying reusable interrupts relies on highly architecture-specific structures).

I think I had also initially been seeing hints part of the memory handling might remain in xen_intr.c, but that may have disappeared with PVHv1.

[ehem_freebsd_m5p.com]

sys/xen/arch-intr.h

63–65 ↗

(On Diff #91464)

So this really does look like a good question of what these should look like. The minimum arguments are:

struct xenisrc *xen_arch_intr_alloc(enum evtchn_type type, evtchn_port_t port);
void    xen_arch_intr_release(struct xenisrc *isrc);

Testing confirmed passing the name is actually unnecessary. Meanwhile, if the architecture file takes ownership of M_XENINTR then that can be static to architecture and need not be passed around. Checking found the other use of M_XENINTR disappeared from with D30228.

Thoughts? (somehow I'm guessing those are going to be "do it")

[ehem_freebsd_m5p.com]

Another possible prototype might be struct xenisrc *xen_arch_intr_alloc(const struct xenisrc *params);. This has the advantage of xen_intr_bind_isrc() could merge its type and port parameters together. Better yet, if xen_intr_bind_virq() set xi_virq then arm64 could use these in the intr_event_create() call (for virqs I would tend towards "xen-virq-%u" as the format).

Then the architecture functions simply do memcpy(&isrc->xi_arch + 1, &params->xi_arch + 1, sizeof(struct xenisrc) - sizeof(xen_arch_isrc_t));, to copy everything from the passed-in temporary structure to the final structure.

[ehem_freebsd_m5p.com]

Update to reflect adjustment to D30726.

[ehem_freebsd_m5p.com]

Updating diff to match with D30726 update.

[ehem_freebsd_m5p.com]

Updating diff to match other diffs. Deltas which overlap can be trouble.

[ehem_freebsd_m5p.com]

Update reflecting logic fix to D30909. The return of xen_arch_intr_has_handlers() is inverted from what had previously been used.

[ehem_freebsd_m5p.com]

sys/x86/xen/xen_arch_intr.c

406–408

Mentioned on D30726, should this segment be dropped? These values must be set during the allocation process, while the architecture code only looks at ->xi_type (== EVTCHN_TYPE_UNBOUND, presently unallocated, other values indicate in use). Though looks like xen_intr_bind_isrc() may need to start clearing ->xi_cookie (which it should have been doing already, D31188 implements this implicitly).

sys/xen/arch-intr.h

63–65 ↗

(On Diff #91464)

Though by doing less in D30935 it is possible to remove the requirement for passing the port (leaving the xen_intr_assign_cpu() in xen_intr_bind_isrc()). Alternatively D31188 passes the port (and everything else; this in fact was the inspiration for D31188).

[ehem_freebsd_m5p.com]

Updating to present status. Difficult keeping everything up to date when you've got >60 diffs and ~100 commits in the queue.