On Wed, Jan 08, 2020 at 04:23:25PM +0100, Thomas Gleixner wrote:
> Anchal Agarwal <anchalag@xxxxxxxxxx> writes:
>
> > shutdown_pirq is invoked during hibernation path and hence
> > PIRQs should be restarted during resume.
> > Before this commit'020db9d3c1dc0a' xen/events: Fix interrupt lost
> > during irq_disable and irq_enable startup_pirq was automatically
> > called during irq_enable however, after this commit pirq's did not
> > get explicitly started once resumed from hibernation.
> >
> > chip->irq_startup is called only if IRQD_IRQ_STARTED is unset during
> > irq_startup on resume. This flag gets cleared by free_irq->irq_shutdown
> > during suspend. free_irq() never gets explicitly called for ioapic-edge
> > and ioapic-level interrupts as respective drivers do nothing during
> > suspend/resume. So we shut them down explicitly in the first place in
> > syscore_suspend path to clear IRQ<>event channel mapping. shutdown_pirq
> > being called explicitly during suspend does not clear this flags, hence
> > .irq_enable is called in irq_startup during resume instead and pirq's
> > never start up.
>
> What?
>
> > +void irq_state_clr_started(struct irq_desc *desc)
> > {
> > irqd_clear(&desc->irq_data, IRQD_IRQ_STARTED);
> > }
> > +EXPORT_SYMBOL_GPL(irq_state_clr_started);
>
> This is core internal state and not supposed to be fiddled with by
> drivers.
>
> irq_chip has irq_suspend/resume/pm_shutdown callbacks for a reason.
>
I agree, as its mentioned in the previous patch {[RFC PATCH V2 08/11]} this is
one way of explicitly shutting down legacy devices without introducing too much
code for each of the legacy devices. . for eg. in case of floppy there
is no suspend/freeze handler which should have done the needful.
.
Either we implement them for all the legacy devices that have them missing or
explicitly shutdown pirqs. I have choosen later for simplicity. I understand
that ideally we should enable/disable devices interrupts in suspend/resume
devices but that requires adding code for doing that to few drivers[and I may
not know all of them either]
Now I discovered during the flow in hibernation_platform_enter under resume
devices that for such devices irq_startup is called which checks for
IRQD_IRQ_STARTED flag and based on that it calls irq_enable or irq_startup.
They are only restarted if the flag is not set which is cleared during shutdown.
shutdown_pirq does not do that. Only masking/unmasking of evtchn does not work
as pirq needs to be restarted.
xen-pirq.enable_irq is called rather than stratup_pirq. On resume if these pirqs
are not restarted in this case ACPI SCI interrupts, I do not see receiving
any interrupts under cat /proc/interrupts even though host keeps generating
S4 ACPI events.
Does that makes sense?
Thanks,
Anchal
> Thanks,
>
> tglx
