Page MenuHomeFreeBSD

xen/inter: merge CPU assignment during port binding together
Needs ReviewPublic

Authored by ehem_freebsd_m5p.com on Nov 6 2021, 1:10 AM.

Details

Reviewers
royger
mhorne
Summary

Processor assignments for PORT, VIRQ and IPI interrupts were all being
handled separately. Merge these together to shrink the code. Using the
MI API for IPI interrupts had been lost, and it had never been used for
PORT interrupts. Fix this issue.

Diff Detail

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

Event Timeline

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
426–449

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.

1102

Now that is a funky line attribution for a diff.

1141

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.

sys/xen/xen_intr.c
436–440

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.

sys/xen/xen_intr.c
436–440

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.

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.

Updating to fix potential, unused, but set warning.

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.