On Wed, 2025-04-30 at 14:31 +0200, Niklas Cassel wrote:
> While the parameter 'interrupts' to the functions pci_epc_set_msi()
> and
> pci_epc_set_msix() represent the actual number of interrupts, and
> pci_epc_get_msi() and pci_epc_get_msix() return the actual number of
> interrupts.
>
> These endpoint library functions just mentioned will however supply
> "interrupts - 1" to the EPC callback functions pci_epc_ops->set_msi()
> and
> pci_epc_ops->set_msix(), and likewise add 1 to return value from
> pci_epc_ops->get_msi() and pci_epc_ops->get_msix(), even though the
> parameter name for the callback function is also named 'interrupts'.
>
> While the set_msix() callback function in pcie-designware-ep writes
> the
> Table Size field correctly (N-1), the calculation of the PBA offset
> is wrong because it calculates space for (N-1) entries instead of N.
>
> This results in e.g. the following error when using QEMU with PCI
> passthrough on a device which relies on the PCI endpoint subsystem:
> failed to add PCI capability 0x11[0x50]@0xb0: table & pba overlap, or
> they don't fit in BARs, or don't align
>
> Fix the calculation of PBA offset in the MSI-X capability.
>
> Fixes: 83153d9f36e2 ("PCI: endpoint: Fix ->set_msix() to take BIR and
> offset as arguments")
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> drivers/pci/controller/dwc/pcie-designware-ep.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c
> b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 1a0bf9341542..24026f3f3413 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -585,6 +585,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> *epc, u8 func_no, u8 vfunc_no,
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct dw_pcie_ep_func *ep_func;
> u32 val, reg;
> + u16 actual_interrupts = interrupts + 1;
>
> ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> if (!ep_func || !ep_func->msix_cap)
> @@ -595,7 +596,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> *epc, u8 func_no, u8 vfunc_no,
> reg = ep_func->msix_cap + PCI_MSIX_FLAGS;
> val = dw_pcie_ep_readw_dbi(ep, func_no, reg);
> val &= ~PCI_MSIX_FLAGS_QSIZE;
> - val |= interrupts;
> + val |= interrupts; /* 0's based value */
> dw_pcie_writew_dbi(pci, reg, val);
>
> reg = ep_func->msix_cap + PCI_MSIX_TABLE;
> @@ -603,7 +604,7 @@ static int dw_pcie_ep_set_msix(struct pci_epc
> *epc, u8 func_no, u8 vfunc_no,
> dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> reg = ep_func->msix_cap + PCI_MSIX_PBA;
> - val = (offset + (interrupts * PCI_MSIX_ENTRY_SIZE)) | bir;
> + val = (offset + (actual_interrupts * PCI_MSIX_ENTRY_SIZE)) |
> bir;
> dw_pcie_ep_writel_dbi(ep, func_no, reg, val);
>
> dw_pcie_dbi_ro_wr_dis(pci);
Reviewed-by: Wilfred Mallawa <wilfred.mallawa@xxxxxxx>
Wilfred
