linux-kernel - Re: [PATCH] PCI: Mediatek: clear irq status after irq dispathed to avoid reentry

Open Source and information security mailing list archives

Message-ID: <1513820234.25872.1.camel@mhfsdcap03>
Date:   Thu, 21 Dec 2017 09:37:14 +0800
From:   Honghui Zhang <honghui.zhang@...iatek.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
CC:     <bhelgaas@...gle.com>, <matthias.bgg@...il.com>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-mediatek@...ts.infradead.org>, <linux-pci@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <devicetree@...r.kernel.org>,
        <yingjoe.chen@...iatek.com>, <eddie.huang@...iatek.com>,
        <ryder.lee@...iatek.com>, <youlin.pei@...iatek.com>,
        <hongkun.cao@...iatek.com>, <sean.wang@...iatek.com>,
        <xinping.qian@...iatek.com>, <yt.shen@...iatek.com>,
        <yong.wu@...iatek.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Subject: Re: [PATCH] PCI: Mediatek: clear irq status after irq dispathed to
 avoid reentry

On Wed, 2017-12-20 at 18:52 -0600, Bjorn Helgaas wrote:
> [+cc Lorenzo, please cc him on all drivers/{host,dwc,endpoint} material]
> 
> Please run "git log --oneline drivers/pci/host/pcie-mediatek.c" and
> make yours match.  Same capitalization, same sentence structure, etc.,
> e.g.,
> 
>   PCI: mediatek: Clear IRQ status ...
> 
> s/dispathed/dispatched/
> 
> s/irq/IRQ/ in the summary and all the English text below.

Thanks for your review and sorry for the mismatch, I will fix that in
next version.

> 
> On Wed, Dec 20, 2017 at 10:52:14AM +0800, honghui.zhang@...iatek.com wrote:
> > From: Honghui Zhang <honghui.zhang@...iatek.com>
> > 
> > There maybe a same irq reentry scenario after irq received in current
> > irq handle flow:
> > 	EP device		PCIe host driver	EP driver
> > 1. issue an irq
> > 			2. received irq
> > 			3. clear irq status
> > 			4. dispatch irq
> > 						5. clear irq source
> > The irq status was not successfully cleared at step 2 since the irq
> > source was not cleared yet. So the PCIe host driver may receive the
> > same irq after step 5. Then there's an irq reentry occurred.
> > Even worse, if the reentry irq was not an irq that EP driver expected,
> > it may not handle the irq. Then we may run into the dead loop from
> 
> By "dead loop" I assume you mean "infinite loop"?  I don't think it's
> a deadlock since nothing is waiting.
> 
Yes, it should be "infinite loop", I will update the commit message in
next version.

thanks.
> > step 2 to step 4.
> > Clear the irq status after irq have been dispatched to avoid the irq
> > reentry.
> > 
> > Signed-off-by: Honghui Zhang <honghui.zhang@...iatek.com>
> > ---
> >  drivers/pci/host/pcie-mediatek.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/pci/host/pcie-mediatek.c b/drivers/pci/host/pcie-mediatek.c
> > index db93efd..3248771 100644
> > --- a/drivers/pci/host/pcie-mediatek.c
> > +++ b/drivers/pci/host/pcie-mediatek.c
> > @@ -605,11 +605,11 @@ static irqreturn_t mtk_pcie_intr_handler(int irq, void *data)
> >  
> >  	while ((status = readl(port->base + PCIE_INT_STATUS)) & INTX_MASK) {
> >  		for_each_set_bit_from(bit, &status, PCI_NUM_INTX + INTX_SHIFT) {
> > -			/* Clear the INTx */
> > -			writel(1 << bit, port->base + PCIE_INT_STATUS);
> >  			virq = irq_find_mapping(port->irq_domain,
> >  						bit - INTX_SHIFT);
> >  			generic_handle_irq(virq);
> > +			/* Clear the INTx */
> > +			writel(1 << bit, port->base + PCIE_INT_STATUS);
> >  		}
> >  	}
> >  
> > @@ -619,10 +619,10 @@ static irqreturn_t mtk_pcie_intr_handler(int irq, void *data)
> >  
> >  			while ((imsi_status = readl(port->base + PCIE_IMSI_STATUS))) {
> >  				for_each_set_bit(bit, &imsi_status, MTK_MSI_IRQS_NUM) {
> > -					/* Clear the MSI */
> > -					writel(1 << bit, port->base + PCIE_IMSI_STATUS);
> >  					virq = irq_find_mapping(port->msi_domain, bit);
> >  					generic_handle_irq(virq);
> > +					/* Clear the MSI */
> > +					writel(1 << bit, port->base + PCIE_IMSI_STATUS);
> >  				}
> >  			}
> >  			/* Clear MSI interrupt status */
> > -- 
> > 2.6.4
> > 
> > 
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

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