On 09/05/16 16:44, Jon Hunter wrote:

On 09/05/16 16:10, Marc Zyngier wrote:

quoted

On 09/05/16 14:13, Jon Hunter wrote:

quoted

On 09/05/16 13:23, Marc Zyngier wrote:

[snip]

quoted

quoted

quoted

This patch have the effect of making misconfigured PPIs absolutely
obvious. I still need to wrap my head around the root cause, but here's
the findings I have so far:

- kvmtool generates a DT with the wrong trigger information (edge
instead of level) for the timer.
- with this patch applied, "cyclictest -S" reliably locks up when run in
a guest (missing a timer interrupt, goodbye CPU).
- Either fixing kvmtool or reverting that patch makes it work reliably
again.

My gut feeling is that until that patch, the failing irq_set_irq_type()
wasn't affecting the kernel's view of the trigger (it was still treated
as level). With this patch, the kernel now trusts whatever is coming
from the firmware, and the misconfiguration becomes obvious. And just
grepping through the DT files for arm and arm64 sends makes me thing
"Holly effin' crap!".

I'm not saying that we shouldn't perform this change though. But it is
quite obvious that it is going to break an awful lot of existing code
and platforms. I'm also cooking a small patch for the arch timer (which
seems to be described in DT with a fairly high level of brokenness), so
that we can mop-up most of the brain damage.

Hmmm ... yes I see. I wonder if we should make the setting of the type
here dependent upon PM being enabled for an irqchip? We could check to
see if the .parent_device is populated and if so only then save the type
and otherwise just set it as we do today.

I don't really like the idea of having multiple code paths for the same thing.
This is very error prone, and likely to bitrot pretty quickly.

True. However, we really need this change for irqchips and runtime-pm.
So to confirm what are you suggesting we do? We could add a WARN around
irq_set_irq_type() in irq_create_fwspec_mapping() for v4.7 and see how
many complaints we get :-)

Let's add a pr_warn(), and see how noisy this becomes. We may have to
remove it if it becomes too loud though.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html