linux-kernel - Re: [PATCH] pinctrl: cherryview: fix issues caused by dynamic gpio irqs mapping

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1ff05c6b-9bac-b8e7-fb10-cd146e4e8bf8@ti.com>
Date:   Tue, 3 Oct 2017 13:20:05 -0500
From:   Grygorii Strashko <grygorii.strashko@...com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
CC:     Linus Walleij <linus.walleij@...aro.org>,
        "linux-gpio@...r.kernel.org" <linux-gpio@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Chris Gorman <chrisjohgorman@...il.com>,
        Mika Westerberg <mika.westerberg@...ux.intel.com>,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        <theadamlevy@...il.com>
Subject: Re: [PATCH] pinctrl: cherryview: fix issues caused by dynamic gpio
 irqs mapping



On 10/03/2017 12:54 PM, Andy Shevchenko wrote:
> On Tue, Oct 3, 2017 at 8:00 PM, Grygorii Strashko
> <grygorii.strashko@...com> wrote:
>> New GPIO IRQs are allocated and mapped dynamically by default when
>> GPIO IRQ infrastructure is used by cherryview-pinctrl driver.
>> This causes issues on some Intel platforms [1][2] with broken BIOS which
>> hardcodes Linux IRQ numbers in their ACPI tables.
>>
>> On such platforms cherryview-pinctrl driver should allocate and map all
>> GPIO IRQs at probe time.
>> Side effect - "Cannot allocate irq_descs @ IRQ%d, assuming pre-allocated\n"
>> can be seen at boot log.
>>
>> NOTE. It still may fail if boot sequence will changed and some interrupt
>> controller will be probed before cherryview-pinctrl which will shift Linux IRQ
>> numbering (expected with CONFIG_SPARCE_IRQ enabled).
>>
>> [1] https://bugzilla.kernel.org/show_bug.cgi?id=194945
>> [2] https://lkml.org/lkml/2017/9/28/153
> 
> Btw, you might want to use one of
> 
> Buglink:
> Bugzilla:
> Link:
> 
> tags.
> 
>> Cc: Andy Shevchenko <andy.shevchenko@...il.com>
>> Cc: Chris Gorman <chrisjohgorman@...il.com>
>> Cc: Mika Westerberg <mika.westerberg@...ux.intel.com>
>> Cc: Heikki Krogerus <heikki.krogerus@...ux.intel.com>
>> Signed-off-by: Grygorii Strashko <grygorii.strashko@...com>
>> Reported-by: Chris Gorman <chrisjohgorman@...il.com>
>> Reported-by: Mika Westerberg <mika.westerberg@...ux.intel.com>
> 
> I'm not sure about this approach. It might be some other broken BIOS
> discovered with some other driver (like pinctrl-baytrail.c as an
> hypothetical example).

It can be made more safe by fixing gpio chip base irq numbers, but this info
should be passed to drivers somehow. Of course, above note will be still valid
- as W/A, required IRQ ranges can be reserved very early during boot in platform code.

> 
> Alternative is to deselect SPARSE_IRQ (though it makes it non-usable
> in entire x86 world).

:) nuke mix.

> 
>>   drivers/pinctrl/intel/pinctrl-cherryview.c | 14 +++++++++++++-
>>   1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/pinctrl/intel/pinctrl-cherryview.c b/drivers/pinctrl/intel/pinctrl-cherryview.c
>> index 04e929f..fadbca9 100644
>> --- a/drivers/pinctrl/intel/pinctrl-cherryview.c
>> +++ b/drivers/pinctrl/intel/pinctrl-cherryview.c
>> @@ -1577,6 +1577,7 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>>          struct gpio_chip *chip = &pctrl->chip;
>>          bool need_valid_mask = !dmi_check_system(chv_no_valid_mask);
>>          int ret, i, offset;
>> +       int irq_base;
>>
>>          *chip = chv_gpio_chip;
>>
>> @@ -1622,7 +1623,18 @@ static int chv_gpio_probe(struct chv_pinctrl *pctrl, int irq)
>>          /* Clear all interrupts */
>>          chv_writel(0xffff, pctrl->regs + CHV_INTSTAT);
>>
>> -       ret = gpiochip_irqchip_add(chip, &chv_gpio_irqchip, 0,
>> +       if (!need_valid_mask) {
>> +               irq_base = devm_irq_alloc_descs(pctrl->dev, -1, 0,
>> +                                               chip->ngpio, NUMA_NO_NODE);
>> +               if (irq_base < 0) {
>> +                       dev_err(pctrl->dev, "Failed to allocate IRQ numbers\n");
>> +                       return irq_base;
>> +               }
>> +       } else {
>> +               irq_base = 0;
>> +       }
>> +
>> +       ret = gpiochip_irqchip_add(chip, &chv_gpio_irqchip, irq_base,
>>                                     handle_bad_irq, IRQ_TYPE_NONE);
>>          if (ret) {
>>                  dev_err(pctrl->dev, "failed to add IRQ chip\n");
>> --
>> 2.10.1
>>
> 
> 
> 

-- 
regards,
-grygorii