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

[Manikanta Maddireddy <mmaddireddy@nvidia.com>]

On 15/10/26 11:13 pm, Koichiro Den wrote:
> On Wed, Oct 14, 2026 at 08:10:32PM +0530, Manivannan Sadhasivam wrote:
>> 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@nvidia.com>
>>>> ---
>>>>   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.
> 
> I wonder if nvmet-pci-epf was one of the cases Niklas had in mind.
> Patch #10 sets .msix_capable = true, so with msix_interrupts set to a non-zero
> value, nvmet_pci_epf_init_irq() selects MSI-X and calls pci_epc_set_msix() with
> BAR0, which looks like the concern this patch #11 itself wants to resolve.
> 
> So, Manikanta, have you tested nvmet-pci-epf with MSI-X on Tegra with and
> without this series? I don't have the hardware, so please correct me if I'm
> missing something.
> 
>>
>>>
>>> Also, if you run then pci_endpoint kselftest, with the default 2048 MSI-X
>>> interrupts. Are really all 2048 MSI-X successful?
> 
> This also reminds me of the unresolved entry #0 issue Niklas and I saw on
> RK3588... The kselftest's final status hid the failure when a later vector was
> skipped, so it's worth checking the individual results. If the same symptom
> appears on Tegra, it would be useful to compare the two.
> 
>>>
>>> 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@valinux.co.jp/
>>>
>>> +	if (layout->table_size < table_size || layout->pba_size < pba_size)
>>> +		return -ENOSPC;
> 
> Unlike my previous series, this patch skips pci_epc_set_msix(), so as I
> understand it, msix_interrupts does not change the Table Size advertised to the
> host. Assuming the existing Table Size matches the hardware, that would avoid
> advertising more entries than it has.
> 
> (BTW, skipping pci_epc_set_msix() alone would not be enough for RK3588. Its
> upstream driver does not yet advertise the fixed MSI-X Table/PBA regions, so
> this patch does not change its behaviour. But if we add those regions later, the
> interrupt path may need to change too. dw_pcie_ep_raise_msix_irq() reads an
> EPF-owned Table through ep_func->epf_bar[], which is not populated for the
> reserved BAR4. The rockchip driver would need to select the controller doorbell
> when using the fixed Table.)
> 
> Best regards,
> Koichiro Den

I verified only with pci-epf-test driver.

Tegra MSI-X HW supports maximum 8 vectors. pci_endpoint_test drivers 
supports MSI-X allocation from 1 to 2048, so this driver is allocating 8 
MSI-X vectors and I could see 8 MSI-X vector and the count increase on 
all 8 vectors.

  152:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    0 Edge      pci-endpoint-test.0
  153:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    1 Edge      pci-endpoint-test.0
  154:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    2 Edge      pci-endpoint-test.0
  155:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    3 Edge      pci-endpoint-test.0
  156:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    4 Edge      pci-endpoint-test.0
  157:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    5 Edge      pci-endpoint-test.0
  158:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    6 Edge      pci-endpoint-test.0
  159:          2          0          0          0          0          0 
          0          0          0          0          0          0 
DW-PCI-MSIX-0005:01:00.0    7 Edge      pci-endpoint-test.0


For now, I will drop msix feature from this series.

> 
>>>
>>>
>>>
>>> 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
>>

-- 
nvpublic