xen/intr: merge CPU assignment during port binding together

[ehem_freebsd_m5p.com]

Summary

Processor assignments for PORT, VIRQ and IPI interrupts were all being
handled separately. Merge these together to shrink the code.

PORT channels previously hadn't been attempting intr_event_bind(). This
appears to have been missed during development, so add this to ensure
consistency with other event channel types.

Due to needing to recover from intr_event_bind() failing,
xen_intr_bind_virq() and xen_intr_alloc_and_bind_ipi() could call
xen_intr_unbind(). With the intr_event_bind() call removed there is no
longer any potential need to call xen_intr_unbind().

Fixes: f229f35db70f ("xen/intr: balance dynamic interrupts across available vCPUs")

[ehem_freebsd_m5p.com]

Situation of exploring the effects of D31188. I rather like D31188's effect on the interface, but that needs a review before I'm going to start assuming things will head this direction.

sys/xen/xen_intr.c

416–423 ↗

(On Diff #98109)

If D31188 and D32866 were approved quickly, D30935 would instead merely move this piece to xen_intr_alloc_isrc(). This would reduce code duplication since no architecture would need to call xen_intr_assign_cpu(). Instead each architecture would simply assign ->xi_cpu and this would take care of the assignment.

This is a more advanced version of what I was referring to in my recent comment on D30935.

1085 ↗

(On Diff #98109)

Now that is a funky line attribution for a diff.

1124 ↗

(On Diff #98109)

Before ca7af67ac958d3749ebfc4771cecf0cd7525e6a0 there was:

if (error == 0)
        error = intr_event_bind(isrc->xi_intsrc.is_event, cpu);

Here, but that got lost. I believe this is a small commit goof, so I believe doing intr_event_bind() (or xen_arch_intr_event_bind()) before the xen_intr_assign_cpu() is the correct. Otherwise, removing the intr_event_bind()/xen_arch_intr_event_bind() call for all cases is correct.

[ehem_freebsd_m5p.com]

sys/xen/xen_intr.c

427–431 ↗

(On Diff #98109)

What did I learn when looking at history? Apparently not having this for PORT channels was a deliberate performance decision. That seems odd, but was apparently found worthwhile.

In such case this segment should have a if (isrc->xi_type != EVTCHN_TYPE_PORT) { added around it. This retains the advantage of doing processor assignments for all interrupts during initialization and in the same location.

[ehem_freebsd_m5p.com]

sys/xen/xen_intr.c

427–431 ↗

(On Diff #98109)

Then read again and the commit messages don't state such.

The one for 76acc41fb7c740bc49e2638529b8cc750ff281d5 is more concerned about avoiding the APICs (this retains that). Meanwhile when this was implemented at f229f35db70f9a4bfac12c49e91bd9dd56a4975e there is no reference.

I'm reverting to this was simply missed during implementation for PORT channels.

[ehem_freebsd_m5p.com]

Fallout from updates to D30743. The update makes the original patch no longer apply cleanly. Since I expect D30743 to go in first, ensure it works.

[ehem_freebsd_m5p.com]

Updating to fix potential, unused, but set warning.

[ehem_freebsd_m5p.com]

Updating to match present repository status. Still the same underlying approach, but somewhat refined. Also adjusted due to other changes being moved aside for now.

[ehem_freebsd_m5p.com]

Updating to current tree status.

[ehem_freebsd_m5p.com]

Updating Phabricator due to discussions elsewhere. I suspect this will get further adjustment.