Page MenuHomeFreeBSD

xen/intr: add check for intr_register_source() errors
Needs ReviewPublic

Authored by ehem_freebsd_m5p.com on Sep 16 2021, 10:53 PM.

Details

Reviewers
royger
mhorne
Summary

While unusual, intr_register_source() can return failure. A likely
cause might be another device grabbing from Xen's interrupt range.
This should NOT happen, but could happen due to a bug. As such check
for this and fail if it occurs.

This also covers the condition identified by the KASSERT(), so remove
the now redundant KASSERT().

Diff Detail

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

Event Timeline

This really doesn't fit with D30743. This one is checking for this one situation which should never occur, but reality is perverse.

Most likely trigger is intr_register_source() does a check for intr_lookup_source(vector) and will return EEXIST for the situation identical to the KASSERT(). This though would require something else to grab an interrupt from the Xen code's range; which in turn shouldn't happen.

This doesn't work as part of D30743, since this isn't focused on xen_intr_port_to_isrc[]. This is simply a check which is really needed. Isn't needed as part of the Xen/ARM64 project, just my entire patch series includes this. This shouldn't occur, but safety checks are for the conditions we don't expect to occur.

Looks fine to me. I would suggest keeping the KASSERT though, it is free and makes the point of failure more obvious if this condition did occur.

This revision is now accepted and ready to land.Sep 17 2021, 12:27 PM

On review, at HEAD the logic is inverted. When returning with NULL, the lock needs to be held. Updating to match actual situation.

This revision now requires review to proceed.Sat, Oct 23, 8:56 PM

If you're wondering about D30726 referencing the call structures being perverse, this is one of the spots. Alternatively due to commit reordering this got reordered before D30726 which flips the logic. This is what should be done if before D30726.