linux-kernel - Re: [PATCH 05/15] PCI: mvebu: Disallow mapping interrupts on emulated bridges

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: <20220107221348.5s4ehqhxunmoacbm@pali>
Date:   Fri, 7 Jan 2022 23:13:48 +0100
From:   Pali Rohár <pali@...nel.org>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
        Lorenzo Pieralisi <lorenzo.pieralisi@....com>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Wilczyński <kw@...ux.com>,
        Bjorn Helgaas <bhelgaas@...gle.com>,
        Marek Behún <kabel@...nel.org>,
        linux-pci@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 05/15] PCI: mvebu: Disallow mapping interrupts on
 emulated bridges

On Friday 07 January 2022 15:32:16 Bjorn Helgaas wrote:
> On Thu, Nov 25, 2021 at 01:45:55PM +0100, Pali Rohár wrote:
> > Interrupt support on mvebu emulated bridges is not implemented yet.
> 
> Is this mvebu-specific, or is aardvar also affected?

This is pci-mvebu.c driver specific, it does not implement emulation of
neither INTx, nor MSI interrupts for emulated pci bridge (root port). As
we know this HW does not have compliant pci root port, it needs to be
emulated in driver, and emulation for interrupts is missing. (it means
that also AER interrupt is missing).

And pci-aardvark.c driver has same issue and similar patch is required
for pci-aardvark.c too. Marek should take care of it. But for
pci-aardvark we already have implementation which emulates INTx
interrupts and it is waiting for review on the list:
https://lore.kernel.org/linux-pci/20211208061851.31867-1-kabel@kernel.org/

> > So properly indicate return value to callers that they cannot request
> > interrupts from emulated bridge.
> 
> Pet peeve: descriptions that say "do this *properly*".  As though the
> previous authors were just ignorant or intentionally did something
> *improperly* :)
> 
> > Signed-off-by: Pali Rohár <pali@...nel.org>
> > Cc: stable@...r.kernel.org
> > ---
> >  drivers/pci/controller/pci-mvebu.c | 10 ++++++++++
> >  1 file changed, 10 insertions(+)
> > 
> > diff --git a/drivers/pci/controller/pci-mvebu.c b/drivers/pci/controller/pci-mvebu.c
> > index 19c6ee298442..a3df352d440e 100644
> > --- a/drivers/pci/controller/pci-mvebu.c
> > +++ b/drivers/pci/controller/pci-mvebu.c
> > @@ -705,6 +705,15 @@ static struct pci_ops mvebu_pcie_ops = {
> >  	.write = mvebu_pcie_wr_conf,
> >  };
> >  
> > +static int mvebu_pcie_map_irq(const struct pci_dev *dev, u8 slot, u8 pin)
> > +{
> > +	/* Interrupt support on mvebu emulated bridges is not implemented yet */
> > +	if (dev->bus->number == 0)
> > +		return 0; /* Proper return code 0 == NO_IRQ */
> > +
> > +	return of_irq_parse_and_map_pci(dev, slot, pin);
> 
> Is this something that could be done with a .read_base() op, e.g.,
> make PCI_INTERRUPT_PIN contain zero (PCI_INTERRUPT_UNKNOWN)?

I'm not sure... maybe. I choose this style as after I implement
emulation of INTx interrupts it allows me just to replace "return 0;" by
"return my_mapping_function_for_root_port(...);". Similarly like it is
in pending pci-aardvark.c patch:
https://lore.kernel.org/linux-pci/20211208061851.31867-15-kabel@kernel.org/

> > +}
> > +
> >  static resource_size_t mvebu_pcie_align_resource(struct pci_dev *dev,
> >  						 const struct resource *res,
> >  						 resource_size_t start,
> > @@ -1119,6 +1128,7 @@ static int mvebu_pcie_probe(struct platform_device *pdev)
> >  	bridge->sysdata = pcie;
> >  	bridge->ops = &mvebu_pcie_ops;
> >  	bridge->align_resource = mvebu_pcie_align_resource;
> > +	bridge->map_irq = mvebu_pcie_map_irq;
> 
> I assume this means INTx doesn't work for some devices?  Which ones?
> I guess anything on the root bus?  But INTx for devices *below* these
> emulated Root Ports *does* work?

Exactly. All devices except emulated root ports (which are on bus 0)
have working MSI, MSI-X and INTx interrupts.

> >  	return pci_host_probe(bridge);
> >  }
> > -- 
> > 2.20.1
> >