linux-kernel - RE: [PATCH 2/3] aspeed-video: clear spurious interrupt bits unconditionally

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: <HK0PR06MB338018668005D679C51EF69AF2DE0@HK0PR06MB3380.apcprd06.prod.outlook.com>
Date:   Wed, 23 Dec 2020 02:53:33 +0000
From:   Ryan Chen <ryan_chen@...eedtech.com>
To:     Joel Stanley <joel@....id.au>, Zev Weiss <zev@...ilderbeest.net>
CC:     Eddie James <eajames@...ux.ibm.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Andrew Jeffery <andrew@...id.au>,
        "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        OpenBMC Maillist <openbmc@...ts.ozlabs.org>,
        Linux ARM <linux-arm-kernel@...ts.infradead.org>,
        linux-aspeed <linux-aspeed@...ts.ozlabs.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jae Hyun Yoo <jae.hyun.yoo@...ux.intel.com>
Subject: RE: [PATCH 2/3] aspeed-video: clear spurious interrupt bits
 unconditionally

> -----Original Message-----
> From: Joel Stanley <joel@....id.au>
> Sent: Wednesday, December 23, 2020 9:07 AM
> To: Zev Weiss <zev@...ilderbeest.net>; Ryan Chen
> <ryan_chen@...eedtech.com>
> Cc: Eddie James <eajames@...ux.ibm.com>; Mauro Carvalho Chehab
> <mchehab@...nel.org>; Andrew Jeffery <andrew@...id.au>;
> linux-media@...r.kernel.org; OpenBMC Maillist <openbmc@...ts.ozlabs.org>;
> Linux ARM <linux-arm-kernel@...ts.infradead.org>; linux-aspeed
> <linux-aspeed@...ts.ozlabs.org>; Linux Kernel Mailing List
> <linux-kernel@...r.kernel.org>; Jae Hyun Yoo <jae.hyun.yoo@...ux.intel.com>
> Subject: Re: [PATCH 2/3] aspeed-video: clear spurious interrupt bits
> unconditionally
> 
> On Tue, 22 Dec 2020 at 19:14, Zev Weiss <zev@...ilderbeest.net> wrote:
> >
> > On Mon, Dec 21, 2020 at 10:47:37PM CST, Joel Stanley wrote:
> > >On Tue, 15 Dec 2020 at 02:46, Zev Weiss <zev@...ilderbeest.net> wrote:
> > >>
> > >> Instead of testing and conditionally clearing them one by one, we
> > >> can instead just unconditionally clear them all at once.
> > >>
> > >> Signed-off-by: Zev Weiss <zev@...ilderbeest.net>
> > >
> > >I had a poke at the assembly and it looks like GCC is clearing the
> > >bits unconditionally anyway, so removing the tests provides no change.
> > >
> > >Combining them is a good further optimization.
> > >
> > >Reviewed-by: Joel Stanley <joel@....id.au>
> > >
> > >A question unrelated to this patch: Do you know why the driver
> > >doesn't clear the status bits in the interrupt handler? I would
> > >expect it to write the value of sts back to the register to ack the
> > >pending interrupt.
> > >
> >
> > No, I don't, and I was sort of wondering the same thing actually --
> > I'm not deeply familiar with this hardware or driver though, so I was
> > a bit hesitant to start messing with things.  (Though maybe doing so
> > would address the "stickiness" aspect when it does manifest.)  Perhaps
> > Eddie or Jae can shed some light here?
> 
> I think you're onto something here - this would be why the status bits seem to
> stick until the device is reset.
> 
> Until Aspeed can clarify if this is a hardware or software issue, I suggest we ack
> the bits and log a message when we see them, instead of always ignoring them
> without taking any action.
> 
> Can you write a patch that changes the interrupt handler to ack status bits as it
> handles each of them?
> 
Hello Zev, before the patch, do you met issue with irq handler? [continuous incoming?]

In aspeed_video_irq handler should only handle enable interrupt expected.
   u32 sts = aspeed_video_read(video, VE_INTERRUPT_STATUS);
 + sts &= aspeed_video_read(video, VE_INTERRUPT_CTRL);

Ryan

> >
> >
> > Zev
> >