Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [PATCH] irqchip/gic-v3-its: Don't try to move a disabled irq | Date | Tue, 02 Jun 2020 22:54:14 +0200 |
| |
"Herrenschmidt, Benjamin" <benh@amazon.com> writes: > On Sun, 2020-05-31 at 12:09 +0100, Marc Zyngier wrote: >> > The semantic of activate/deactivate (which maps to started/shutdown >> > in the IRQ code) is that the HW resources for a given interrupt are >> > only committed when the interrupt is activated. Trying to perform >> > actions involving the HW on an interrupt that isn't active cannot be >> > guaranteed to take effect. >> > >> > I'd rather address it in the core code, by preventing set_affinity (and >> > potentially others) to take place when the interrupt is not in the >> > STARTED state. Userspace would get an error, which is perfectly >> > legitimate, and which it already has to deal with it for plenty of >> > other >> > reasons. > > So I finally found time to dig a bit in there :) Code has changed a bit > since last I looked. But I have memories of the startup code messing > around with the affinity, and here it is. In irq_startup() : > > > switch (__irq_startup_managed(desc, aff, force)) { > case IRQ_STARTUP_NORMAL: > ret = __irq_startup(desc); > irq_setup_affinity(desc); > break; > case IRQ_STARTUP_MANAGED: > irq_do_set_affinity(d, aff, false); > ret = __irq_startup(desc); > break; > case IRQ_STARTUP_ABORT: > irqd_set_managed_shutdown(d); > return 0; > > So we have two cases here. Normal and managed. > > In the managed case, we set the affinity before startup. I feel like your > patch might break that or am I missing something ?
It will break stuff because the affinity is not stored in case that the interrupt is not started.
I think we can fix this in the core code but that needs more thought. __irq_can_set_affinity() is definitely the wrong place.
Thanks,
tglx
| |