[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <87zhevrupf.fsf@nanos.tec.linutronix.de>
Date: Fri, 10 Jan 2020 20:13:16 +0100
From: Thomas Gleixner <tglx@...utronix.de>
To: Anchal Agarwal <anchalag@...zon.com>
Cc: mingo@...hat.com, bp@...en8.de, hpa@...or.com, x86@...nel.org,
boris.ostrovsky@...cle.com, jgross@...e.com,
linux-pm@...r.kernel.org, linux-mm@...ck.org, kamatam@...zon.com,
sstabellini@...nel.org, konrad.wilk@...cle.co,
roger.pau@...rix.com, axboe@...nel.dk, davem@...emloft.net,
rjw@...ysocki.net, len.brown@...el.com, pavel@....cz,
peterz@...radead.org, eduval@...zon.com, sblbir@...zon.com,
xen-devel@...ts.xenproject.org, vkuznets@...hat.com,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
dwmw@...zon.co.uk, fllinden@...zon.com, anchalag@...zon.com
Subject: Re: [RFC PATCH V2 09/11] xen: Clear IRQD_IRQ_STARTED flag during shutdown PIRQs
Anchal,
Anchal Agarwal <anchalag@...zon.com> writes:
> On Thu, Jan 09, 2020 at 01:07:27PM +0100, Thomas Gleixner wrote:
>> Anchal Agarwal <anchalag@...zon.com> writes:
>> So either you can handle it purely on the XEN side without touching any
>> core state or you need to come up with some way of letting the core code
>> know that it should invoke shutdown instead of disable.
>>
>> Something like the completely untested patch below.
>
> Understandable. Really appreciate the patch suggestion below and i will test it
> for sure and see if things can be fixed properly in irq core if thats the only
> option. In the meanwhile, I tried to fix it on xen side unless it gives you the
> same feeling as above? MSI-x are just fine, just ioapic ones don't get any event
> channel asssigned hence enable_dynirq does nothing. Those needs to be restarted.
>
> diff --git a/drivers/xen/events/events_base.c b/drivers/xen/events/events_base.c
> index 1bb0b522d004..2ed152f35816 100644
> --- a/drivers/xen/events/events_base.c
> +++ b/drivers/xen/events/events_base.c
> @@ -575,6 +575,11 @@ static void shutdown_pirq(struct irq_data *data)
>
> static void enable_pirq(struct irq_data *data)
> {
> +/*ioapic interrupts don't get event channel assigned
> + * after being explicitly shutdown during guest
> + * hibernation. They need to be restarted*/
> + if(!evtchn_from_irq(data->irq))
> + startup_pirq(data);
> enable_dynirq(data);
> }
Interesting patch format :)
Doing the shutdown from syscore_ops and the startup conditionally in a
totaly unrelated function is not really intuitive.
So either you do it symmetrically in XEN via syscore_ops callbacks or
you let the irq core code help you out with the patch I provided
Thanks,
tglx
Powered by blists - more mailing lists