linux-kernel - Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts

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]

Date:   Sat, 12 Feb 2022 10:59:37 +0000
From:   Marc Zyngier <maz@...nel.org>
To:     Lorenzo Pieralisi <lorenzo.pieralisi@....com>
Cc:     Pali Rohár <pali@...nel.org>, robh+dt@...nel.org,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Marek Behún <kabel@...nel.org>,
        Russell King <rmk+kernel@...linux.org.uk>,
        linux-pci@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH v2 10/11] PCI: mvebu: Implement support for legacy INTx interrupts

On Fri, 11 Feb 2022 18:21:37 +0000,
Lorenzo Pieralisi <lorenzo.pieralisi@....com> wrote:
> 
> On Fri, Feb 11, 2022 at 06:52:02PM +0100, Pali Rohár wrote:
> 
> [...]
> 
> > > > @@ -1121,6 +1247,21 @@ static int mvebu_pcie_parse_port(struct mvebu_pcie *pcie,
> > > >  		port->io_attr = -1;
> > > >  	}
> > > >  
> > > > +	/*
> > > > +	 * Old DT bindings do not contain "intx" interrupt
> > > > +	 * so do not fail probing driver when interrupt does not exist.
> > > > +	 */
> > > > +	port->intx_irq = of_irq_get_byname(child, "intx");
> > > > +	if (port->intx_irq == -EPROBE_DEFER) {
> > > > +		ret = port->intx_irq;
> > > > +		goto err;
> > > > +	}
> > > > +	if (port->intx_irq <= 0) {
> > > > +		dev_warn(dev, "%s: legacy INTx interrupts cannot be masked individually, "
> > > > +			      "%pOF does not contain intx interrupt\n",
> > > > +			 port->name, child);
> > > 
> > > Here you end up with a new warning on existing firmware. Is it
> > > legitimate ? I would remove the dev_warn().
> > 
> > I added this warning in v2 because Marc wanted it.
> > 
> > Should I (again) remove it in v3?
> 
> No, I asked a question and gave an opinion, I appreciate Marc's concern
> so leave it (ie not everyone running a new kernel with new warnings on
> existing firmware would be happy - maybe it is a good way of forcing a
> firmware upgrade, you will tell me).

My concern is that short of being able to mask these interrupts, it is
possible for a device to assert an interrupt that no driver handles,
and the kernel spurious interrupt detector won't be able to shut it
up. At this stage, the machine is totally dead (screaming *level*
interrupt).

The dev_warn() could toned down to a dev_warn_once() though.

	M.

-- 
Without deviation from the norm, progress is not possible.