Page MenuHomeFreeBSD

xen/intr: rework xen_intr_resume() for in-place remapping
ClosedPublic

Authored by ehem_freebsd_m5p.com on Jun 1 2021, 3:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Nov 8, 10:22 PM
Unknown Object (File)
Fri, Nov 8, 8:15 PM
Unknown Object (File)
Thu, Oct 31, 3:16 PM
Unknown Object (File)
Wed, Oct 23, 6:47 PM
Unknown Object (File)
Tue, Oct 22, 6:04 PM
Unknown Object (File)
Fri, Oct 18, 11:51 AM
Unknown Object (File)
Thu, Oct 17, 12:30 PM
Unknown Object (File)
Tue, Oct 15, 5:10 AM
Subscribers

Details

Summary

The prior implementation of xen_intr_resume() was wiping
xen_intr_port_to_isrc[] and then rebuilding from the x86 interrupt
table. Rework to instead wipe the channel numbers (->xi_port) and then
scan the table for sources with invalid channels. This removes one more
x86-ism from xen_intr.c, a major benefit.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This is the main goal of D30598/D30599. The variables xen_intr_auto_vector_count and first_evtchn_irq are x86-specific. The call intr_lookup_source() is also x86-specific which is being utilized to get access to interrupt_sources and use that as a table of allocated event channel interrupts. These two make it so xen_intr_port_to_isrc is used for this purpose instead.

xen_intr_auto_vector and first_evtchn_irq are attempting to head back into sys/x86/xen land, while xen_intr_port_to_isrc is definitely going to remain in sys/xen/xen_intr.c. Since the functionality of xen_intr_resume() requires access to xen_intr_port_to_isrc, removing references to xen_intr_auto_vector and first_evtchn_irq pushes the effort to get those out of sys/xen/xen_intr.c forward.

Actually, one question for the FreeBSD/Xen maintainer: Is the order of the xen_intr_port_to_isrc[cur->xi_port] = cur; and evtchn_unmask_port(cur->xi_port); lines important?

I suspect it is since the moment the event channel port is unmasked, events can occur. As such I moved the second call out of xen_intr_rebind_isrc(), but if it is acceptable to have those in the opposite order then having that remain in the function would be nice. I was guessing this would be a race condition and therefore opted for the safe choice.

Add a safety check. Protect against left-behind entries in xen_intr_port_to_isrc[] (shouldn't happen, but just in case).

Comment adjustment, adjustment of tests (consistently use xi_port >= NR_EVENT_CHANNELS).

The key point behind D30598/D30599 is xen_intr_port_to_isrc[] is part of the xen_intr.c file, whereas intr_lookup_source is using the x86-only interrupt_sources[] array for the purpose. As such this removes another x86-ism from the file. I could see these two being squashed in the end.

Tiny update, removing a single line. This line is redundant as the value gets wiped earlier.

Simply updating to match D30598 (just enough delta to confuse patch/git).

Due to this code, one could argue for adding a evtchn_port_t max_port;. The purpose being to keep track of the maximum used port. Before this commit xen_intr_auto_vector_count was being abused for this. Problem of course is at D30936 that variable becomes unavailable and this file needs a separate tracking variable.

Adjust condition to cur->xi_port == isrc_idx. This skips over correct entries, instead of ones with valid xi_port. This potentially steers more cases into the KASSERT().

Small adjustment. This makes the delta smaller, keeping more line in original order. This may simplify retesting. Additionally it is slightly shorter.

Updating patch to ensure successful application with updated D30598.

Tiny update for signed/unsigned issue.

ehem_freebsd_m5p.com retitled this revision from xen/intr: rework xen_intr_resume() to use port mappings as implicit listing to xen/intr: rework xen_intr_resume() for in-place remapping.Nov 25 2021, 6:35 PM
ehem_freebsd_m5p.com edited the summary of this revision. (Show Details)
This revision was not accepted when it landed; it landed in state Needs Review.Mar 29 2023, 7:55 AM
This revision was automatically updated to reflect the committed changes.