On Mon, Oct 15, 2018 at 04:30:35PM -0500, Bjorn Helgaas wrote:
> On Thu, Oct 11, 2018 at 05:16:57PM +0100, Alan Douglas wrote:
> > If EP attempts to send an IRQ (legacy, MSI or MSI-X) while the
> > link is not up, return -EINVAL
> >
> > Fixes: 37dddf14f1ae ("PCI: cadence: Add EndPoint Controller driver for Cadence PCIe controller")
> > Signed-off-by: Alan Douglas <adouglas@xxxxxxxxxxx>
> > ---
> > drivers/pci/controller/pcie-cadence-ep.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
> > index b762214..3667d70 100644
> > --- a/drivers/pci/controller/pcie-cadence-ep.c
> > +++ b/drivers/pci/controller/pcie-cadence-ep.c
> > @@ -370,6 +370,12 @@ static int cdns_pcie_ep_raise_irq(struct pci_epc *epc, u8 fn,
> > u16 interrupt_num)
> > {
> > struct cdns_pcie_ep *ep = epc_get_drvdata(epc);
> > + u32 link_status;
> > +
> > + /* Can't send an IRQ if the link is down. */
> > + link_status = cdns_pcie_readl(&ep->pcie, CDNS_PCIE_LM_BASE);
> > + if (!(link_status & 0x1))
> > + return -EINVAL;
>
> This looks racy. What happens if the link goes down right after the
> CDNS_PCIE_LM_BASE read, but before we get here?
I agree with Bjorn, this check does not seem very useful and honestly
I think this patch should be dropped, I should not have queued it in
the first place.
I wonder whether something can actually be done in the EP subsystem
to handle this in a cleaner way.
Unless I hear a compelling reason to keep it in the patch queue I
would drop this patch.
Lorenzo
