linux-kernel - Re: [PATCH 2/4] PCI: mvebu: Implement support for interrupts on emulated bridge

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]

Date:   Thu, 18 Aug 2022 22:27:24 +0200
From:   Andrew Lunn <andrew@...n.ch>
To:     Pali Rohár <pali@...nel.org>
Cc:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Rob Herring <robh+dt@...nel.org>,
        Sebastian Hesselbarth <sebastian.hesselbarth@...il.com>,
        Gregory Clement <gregory.clement@...tlin.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        linux-pci@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH 2/4] PCI: mvebu: Implement support for interrupts on
 emulated bridge

On Thu, Aug 18, 2022 at 10:07:37PM +0200, Pali Rohár wrote:
> On Thursday 18 August 2022 21:51:21 Andrew Lunn wrote:
> > > -static irqreturn_t mvebu_pcie_irq_handler(int irq, void *arg)
> > > +static irqreturn_t mvebu_pcie_error_irq_handler(int irq, void *arg)
> > > +{
> > > +	struct mvebu_pcie_port *port = arg;
> > > +	struct device *dev = &port->pcie->pdev->dev;
> > > +	u32 cause, unmask, status;
> > > +
> > > +	cause = mvebu_readl(port, PCIE_INT_CAUSE_OFF);
> > > +	unmask = mvebu_readl(port, PCIE_INT_UNMASK_OFF);
> > > +	status = cause & unmask;
> > > +
> > > +	/* "error" interrupt handler does not process INTX interrupts */
> > > +	status &= ~(PCIE_INT_INTX(0) | PCIE_INT_INTX(1) |
> > > +		    PCIE_INT_INTX(2) | PCIE_INT_INTX(3));
> > 
> > Just for my understanding...
> > 
> > There are two interrupts
> 
> yes
> 
> > but the status information what those
> > interrupts actually mean are all packed into one register?
> 
> yes
> 
> for masking individual interrupt events there is just one shared
> register for both "intx" and "error" interrupt source.
> 
> and also there is also just one shared "cause" register which says which
> individual interrupt events happened.
> 
> > I assume reading the clause register does not clear set bits?
> 
> yes, reading does not clear any interrupt event.
> 
> > Otherwise there
> > would be a race condition.
> 
> > Are these actually level interrupts
> 
> yes
> 
> > and in order to clear them you need to poke some other register?
> 
> to clear individual interrupt event you have to write corresponding 1b
> bit into that cause register.
> 
> so if interrupts events BIT(24), BIT(16) and BIT(17) happened and
> BIT(24), BIT(25), BIT(26), BIT(27) and BIT(16) are unmasked then CPU
> receives two interrupts (one for intx:24-27 and one for err:16). kernel
> will call interrupt handlers for both intx and err (possible also in
> parallel if it unmasked on different CPUs) and each handler just clears
> events which process. So writing BIT(16) into cause register clears only
> event 16 and all other (24-27, 17) are still active. And level interrupt
> (the correct one intx or err) is then triggered again.

Thanks for the explanation.

I don't know enough about PCI to be able to give a meaningful
Reviewed-by, so i will leave that to the PCI maintainer. But the DT
bits look good to me.

     Andrew