From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752061AbcEJMU6 (ORCPT ); Tue, 10 May 2016 08:20:58 -0400 Received: from foss.arm.com ([217.140.101.70]:44417 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751347AbcEJMU4 (ORCPT ); Tue, 10 May 2016 08:20:56 -0400 Subject: Re: [PATCH V3 06/17] irqdomain: Don't set type when mapping an IRQ To: Jon Hunter , Thomas Gleixner , Jason Cooper , Rob Herring , Pawel Moll , Mark Rutland , Ian Campbell , Kumar Gala , Stephen Warren , Thierry Reding References: <1462379130-11742-1-git-send-email-jonathanh@nvidia.com> <1462379130-11742-7-git-send-email-jonathanh@nvidia.com> <5730813B.7060206@arm.com> <57308D0D.4080800@nvidia.com> <5730A867.9070504@arm.com> <5730B078.8090908@nvidia.com> Cc: Kevin Hilman , Geert Uytterhoeven , Grygorii Strashko , Lars-Peter Clausen , Linus Walleij , linux-tegra@vger.kernel.org, linux-omap@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org From: Marc Zyngier Organization: ARM Ltd Message-ID: <5731D223.2080503@arm.com> Date: Tue, 10 May 2016 13:20:51 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Icedove/38.7.0 MIME-Version: 1.0 In-Reply-To: <5730B078.8090908@nvidia.com> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 09/05/16 16:44, Jon Hunter wrote: > > On 09/05/16 16:10, Marc Zyngier wrote: >> On 09/05/16 14:13, Jon Hunter wrote: >>> On 09/05/16 13:23, Marc Zyngier wrote: > > [snip] > >>>> 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...