Page MenuHomeFreeBSD

kern: remove clk_intr_event from non-ACPI
Needs ReviewPublic

Authored by ehem_freebsd_m5p.com on Jun 27 2023, 6:53 PM.
Tags
None
Referenced Files
F82792619: D40776.id.diff
Thu, May 2, 3:37 PM
F82792611: D40776.id123860.diff
Thu, May 2, 3:37 PM
F82788651: D40776.diff
Thu, May 2, 3:00 PM
Unknown Object (File)
Sat, Apr 20, 6:57 PM
Unknown Object (File)
Dec 20 2023, 6:17 AM
Unknown Object (File)
Dec 18 2023, 6:30 AM
Unknown Object (File)
Dec 10 2023, 8:33 PM
Unknown Object (File)
Sep 25 2023, 11:52 AM
Subscribers

Details

Reviewers
kib
jhb
mav
mjg
Summary

While there may have been many uses of clk_intr_event in the past, now
the ACPI core is the only use. Remove it from all non-ACPI kernels.

Correct the uses of intr_event_handle(). If no handlers are added by
ACPI, invoking intr_event_handle() is incorrect as it is stray.

Fixes: aba10e131fe ("Allow swi_sched() to be called from NMI context.")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 52301
Build 49192: arc lint + arc unit

Event Timeline

Unfortunately there are several overlapping issues interwoven here and choices need to be made.

  • As @mjg correctly asked in D38448, should interrupts which occur on multiple processors ("PPI" on ARM) share a struct intr_event? This approach is taken by INTRNG and @mav took this approach in D25754 with clk_intr_event. The argument against this approach is IPI and PPI interrupts share characteristics which are not shared with normal interrupts. Notably IPI/PPI need multiple counters or atomics for counters. Additionally normal IPI/PPI only use filters, not handlers. I was starting to think @mjg was correct and they should be split, but D25754 has moved in the direction of merging.
  • clk_intr_event is presently only used by ACPI for ACPI NMI interrupts. D40776 removes it from non-ACPI. Yet there seems merit of going the opposite direction and reimplementing most of hardclock() as filters on clk_intr_event.
  • What triggered D40776 is if clk_intr_event has no handlers (ACPI is absent or lacks the appropriate interrupt types) then intr_event_handle(clk_intr_event, NULL); reports stray interrupts (comment in intr_event_handle() declares these to be stray). Perhaps a flag should be added for indicating a particular intr_event being triggered with no handlers shouldn't be reported as strays? (I had been wondering about a mysterious "swi4:").
  • clk_intr_event has almost no effect on kern_intr.c. I think clk_intr_event needs to be moved to kern_clock.c.

I suspect the first item needs consideration first. Should interrupts expected to occur on multiple processors share a struct intr_event? Things are tending to be combined which heads towards a "yes", but there seem some problematic consequences from this.