Re: [PATCH 11/11] PCI: endpoint: test: Do not relocate fixed MSI-X tables

[Manivannan Sadhasivam]

On 2026-09-23 16:32 +0200, Niklas Cassel wrote:
> On Wed, Sep 23, 2026 at 12:52:37PM +0530, Manikanta Maddireddy wrote:
> > Some endpoint controllers expose MSI-X table/PBA storage at fixed,
> > hardware-owned BARs. Such BARs are described to endpoint functions as
> > BAR_RESERVED with PCI_EPC_BAR_RSVD_MSIX_TBL_RAM, so the function driver
> > must not reprogram the MSI-X Table/PBA capability to point into ordinary
> > function BAR memory.
> > 
> > pci_epf_test always allocates MSI-X storage after its test registers and
> > calls pci_epc_set_msix(). On controllers with fixed MSI-X storage, this
> > overwrites the controller-defined MSI-X layout. For example, the generic
> > DesignWare path derives the PBA offset from the number of vectors, which
> > can make the host program the wrong MSI-X storage and prevent MSI-X
> > delivery.
> > 
> > Detect fixed MSI-X table BARs and leave the controller-owned MSI-X
> > capability layout unchanged. Keep allocating and programming MSI-X
> > storage in the test BAR only for controllers that do not advertise fixed
> > MSI-X table storage.
> > 
> > Signed-off-by: Manikanta Maddireddy <mmaddireddy@xxxxxxxxxx>
> > ---
> >  drivers/pci/endpoint/functions/pci-epf-test.c | 27 +++++++++++++++++--
> >  1 file changed, 25 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> > index d4905aa8e4c0..82d4714e998c 100644
> > --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> > +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> > @@ -83,6 +83,7 @@ struct pci_epf_test {
> >  	struct config_group	group;
> >  	enum pci_barno		test_reg_bar;
> >  	size_t			msix_table_offset;
> > +	bool			msix_table_fixed;
> >  	struct delayed_work	cmd_handler;
> >  	struct dma_chan		*dma_chan_tx;
> >  	struct dma_chan		*dma_chan_rx;
> > @@ -98,6 +99,26 @@ struct pci_epf_test {
> >  	size_t			bar_size[PCI_STD_NUM_BARS];
> >  };
> >  
> > +static bool pci_epf_test_msix_table_fixed(const struct pci_epc_features *features)
> > +{
> > +	enum pci_barno bar;
> > +	int i;
> > +
> > +	for (bar = BAR_0; bar < PCI_STD_NUM_BARS; bar++) {
> > +		const struct pci_epc_bar_desc *desc = &features->bar[bar];
> > +
> > +		if (desc->type != BAR_RESERVED)
> > +			continue;
> > +
> > +		for (i = 0; i < desc->nr_rsvd_regions; i++) {
> > +			if (desc->rsvd_regions[i].type == PCI_EPC_BAR_RSVD_MSIX_TBL_RAM)
> > +				return true;
> > +		}
> > +	}
> > +
> > +	return false;
> > +}
> > +
> >  struct pci_epf_test_reg {
> >  	__le32 magic;
> >  	__le32 command;
> > @@ -1214,7 +1235,7 @@ static int pci_epf_test_epc_init(struct pci_epf *epf)
> >  		}
> >  	}
> >  
> > -	if (epc_features->msix_capable) {
> > +	if (epc_features->msix_capable && !epf_test->msix_table_fixed) {
> >  		ret = pci_epc_set_msix(epc, epf->func_no, epf->vfunc_no,
> >  				       epf->msix_interrupts,
> >  				       epf_test->test_reg_bar,
> > @@ -1282,7 +1303,9 @@ static int pci_epf_test_alloc_space(struct pci_epf *epf)
> >  
> >  	test_reg_bar_size = ALIGN(sizeof(struct pci_epf_test_reg), 128);
> >  
> > -	if (epc_features->msix_capable) {
> > +	epf_test->msix_table_fixed = pci_epf_test_msix_table_fixed(epc_features);
> > +
> > +	if (epc_features->msix_capable && !epf_test->msix_table_fixed) {
> >  		msix_table_size = PCI_MSIX_ENTRY_SIZE * epf->msix_interrupts;
> >  		epf_test->msix_table_offset = test_reg_bar_size;
> >  		/* Align to QWORD or 8 Bytes */
> 
> Here you are only updating pci-epf-test. What about the other EPF drivers?
> 

How can we test those other EPF drivers if there is no hw setup available? TBH,
we don't need to add all functionalities to all EPF drivers. Adding them to
pci-epf-test driver is enough. If other EPF driver maintainers want to add the
missing features, then they can add them after testing on relevant hw setup.

> 
> Also, if you run then pci_endpoint kselftest, with the default 2048 MSI-X
> interrupts. Are really all 2048 MSI-X successful?
> 
> It seems that often the "hardware owned" MSI-X table is much smaller than
> that. See e.g. Koichiro's series which tries to use "hardware owned" MSI-X
> table, but if the "hardware owned" MSI-X table is too small to hold all
> 2048 MSI-X IRQs, then "hardware owned" MSI-X table would not be used:
> 
> https://lore.kernel.org/linux-pci/20260830151948.3547577-2-den@xxxxxxxxxxxxx/
> 
> +	if (layout->table_size < table_size || layout->pba_size < pba_size)
> +		return -ENOSPC;
> 
> 
> 
> Apparently NTB EPF needs to read the MSI-X table from RAM:
> https://lore.kernel.org/linux-pci/pj5vpwwd5tr5tuvhdq3bcpefnq27r6ljvwoysguz2o4tglcqx5@l7zmb6wjnhla/
> 
> Which is why he also decided to let an EPF driver choose if it should
> use "hardware owned" MSI-X table or not.
> 

Yeah, makes sense. Otherwise, we will see kselftests failures.

- Mani