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: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAD=FV=Wg-gdry1a-LjJhuKgHRr=DXq4Hu0P8nJGAzf5viEcthA@mail.gmail.com>
Date:   Tue, 24 Nov 2020 09:01:44 -0800
From:   Doug Anderson <dianders@...omium.org>
To:     Marc Zyngier <maz@...nel.org>
Cc:     Thomas Gleixner <tglx@...utronix.de>,
        Jason Cooper <jason@...edaemon.net>,
        Linus Walleij <linus.walleij@...aro.org>,
        Maulik Shah <mkshah@...eaurora.org>,
        Srinivas Ramana <sramana@...eaurora.org>,
        Neeraj Upadhyay <neeraju@...eaurora.org>,
        Rajendra Nayak <rnayak@...eaurora.org>,
        "open list:GPIO SUBSYSTEM" <linux-gpio@...r.kernel.org>,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        Stephen Boyd <swboyd@...omium.org>,
        Andy Gross <agross@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/3] irqchip: qcom-pdc: Fix phantom irq when changing
 between rising/falling

Hi,

On Tue, Nov 24, 2020 at 1:00 AM Marc Zyngier <maz@...nel.org> wrote:
>
> > @@ -187,9 +189,24 @@ static int qcom_pdc_gic_set_type(struct irq_data
> > *d, unsigned int type)
> >               return -EINVAL;
> >       }
> >
> > +     old_pdc_type = pdc_reg_read(IRQ_i_CFG, pin_out);
> >       pdc_reg_write(IRQ_i_CFG, pin_out, pdc_type);
> >
> > -     return irq_chip_set_type_parent(d, type);
> > +     ret = irq_chip_set_type_parent(d, type);
> > +
> > +     /*
> > +      * When we change types the PDC can give a phantom interrupt.
> > +      * Clear it.  Specifically the phantom shows up if a line is already
> > +      * high and we change to rising or if a line is already low and we
> > +      * change to falling but let's be consistent and clear it always.
> > +      *
> > +      * Doing this works because we have IRQCHIP_SET_TYPE_MASKED so the
> > +      * interrupt will be cleared before the rest of the system sees it.
> > +      */
> > +     if (old_pdc_type != pdc_type)
> > +             irq_chip_set_parent_state(d, IRQCHIP_STATE_PENDING, 0);
>
> nit: s/0/false/.

I'll fix this.


> You could also make it conditional on the parent side having been
> successful.

Good idea.


> And while we're looking at this: do you need to rollback the PDC state
> if the GIC side has failed? It's all very hypothetical, but just in
> case...

I'm going to go ahead and say "no", but I'll make this change if you
insist.  Specifically:

* We're still leaving things in a self-consistent state if we fail to
clear the parent, we'll just get a spurious interrupt.  It won't cause
any crashes / hangs / whatever.

* Since it seems very unlikely we'd ever trip this and if we ever do
it's not the end of the world, I'd rather not add extra code.

Hopefully that's OK.

-Doug

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ