Re: [PATCH 1/2] PCI: endpoint: strictly apply bar fixed size to allocate space

[Niklas Cassel <cassel@kernel.org>]

Hello Jerome,

On Fri, Mar 28, 2025 at 03:53:42PM +0100, Jerome Brunet wrote:
> When trying to allocate space for an endpoint function on a BAR with a
> fixed size, that size should be used regardless of the alignment.

Why?


> 
> Some controller may have specified an alignment, but do have a BAR with a
> fixed size smaller that alignment. In such case, pci_epf_alloc_space()
> tries to allocate a space that matches the alignment and it won't work.

Could you please elaborate "won't work".


> 
> When the BAR size is fixed, pci_epf_alloc_space() should not deviate
> from this fixed size.

I think that this commit is wrong.

In your specific SoC:
 	.msix_capable = false,
 	.bar[BAR_1] = { .type = BAR_RESERVED, },
 	.bar[BAR_3] = { .type = BAR_RESERVED, },
	.bar[BAR_4] = { .type = BAR_FIXED, .fixed_size = 256 },
 	.bar[BAR_5] = { .type = BAR_RESERVED, },
 	.align = SZ_1M,

fixed_size is 256B, inbound iATU alignment is 1 MB, which means that the
smallest area that the iATU can map is 1 MB.

I do think that it makes sense to have backing memory for the whole area
that the iATU will have mapped.


The reason why the the ALIGN() is done, is so that the size sent in to
dma_alloc_coherent() will return addresses that are aligned to the inbound
iATU alignment requirement.


I guess the problem is that your driver has a fixed size BAR that is smaller
than the inbound iATU alignment requirement, something that has never been a
problem before, because no SoC has previously defined such a fixed size BAR.

I doubt the problem is allocating such a BAR, so where is it you actually
encounter a problem? My guess is in .set_bar().

Perhaps the solution is to add another struct member to struct pci_epf_bar,
size (meaning actual BAR size, which will be written to the BAR mask register)
and backing_mem_size.

Or.. we modify pci_epf_alloc_space() to allocate an aligned size, but the
size that we store in (struct pci_epf_bar).size is the unaligned size.


Kind regards,
Niklas