On Thu, Oct 11, 2018 at 05:16:11PM +0100, Alan Douglas wrote:
> The EP driver did not check the mask bit for each MSI before
> sending it in raise_irq. This is now checked, and -EINVAL is
> returned if masked.
>
> 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 | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/drivers/pci/controller/pcie-cadence-ep.c b/drivers/pci/controller/pcie-cadence-ep.c
> index c3a0889..b762214 100644
> --- a/drivers/pci/controller/pcie-cadence-ep.c
> +++ b/drivers/pci/controller/pcie-cadence-ep.c
> @@ -332,6 +332,11 @@ static int cdns_pcie_ep_send_msi_irq(struct cdns_pcie_ep *ep, u8 fn,
> if (!interrupt_num || interrupt_num > msi_count)
> return -EINVAL;
>
> + /* Check whether MSI is masked */
> + data = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_MSI_MASK_64);
> + if (data & (1 << (interrupt_num - 1)))
> + return -EINVAL;
> +
By looking at this, I think this is wrong too. To the best of my
knowledge a masked MSI should be still delivered when the MSI is
unmasked, otherwise the condition that triggered the MSI is lost.
There must be code in the EP subsystem that records the "pending" MSI
and deliver it when the MSI is actually unmasked (I assumed this is
done in HW but I need your clarification on this).
Either that or the return value must signal a masked MSI so that the
code calling send_msi_irq() can reissue it (when though ?).
I will have to drop this patch too unless you convince me I am wrong; I
still want to understand how this code is *currently* working, I
understand that for the time being the only EP device we have is the
test one but this code path does not look very solid to me.
Thanks,
Lorenzo
> /* Compute the data value to be written. */
> data_mask = msi_count - 1;
> data = cdns_pcie_ep_fn_readw(pcie, fn, cap + PCI_MSI_DATA_64);
> --
> 1.9.0
>
