On Tue, Jul 23, 2024 at 09:41:14AM +0200, Rick Wertenbroek wrote: > > Interesting. I like this approach, this would also make it possible to > combine the current 'pci_epf_alloc_space' and 'pci_epc_set_bar' into a > single function which would simplify the endpoint functions. > > The reason why it is separate now is so that 'pci_epf_alloc_space' can > be called before the endpoint function is bound to a controller, and > therefore the BAR memory can be filled and prepared before bind() is > called. Merging 'pci_epf_alloc_bar' with 'pci_epc_set_bar' into a > 'pci_epc_alloc_bar' (epc because it is dependent on the endpoint > controller and prior to be bound to a specific controller we don't > know if we need to allocate locally or remap) would make it impossible > to access the BAR prior to be bound to a controller (because the > function 'pci_epc_alloc_bar' would fail without a specific > controller). I don't think this is a problem as an unbound endpoint > function is kind of useless on its own, but this means the BAR memory > could only be allocated/remapped/accessed once the endpoint is bound > to a controller. Back when I was looking it to this, I came to the conclusion: "The proper place is to allocate them when receiving PERST, but not all drivers have a PERST handler." However, since: https://github.com/torvalds/linux/commit/a01e7214bef904723d26d293eb17586078610379 All EPF drivers should get a epc_init event, either directly when the EPC driver is loaded, or when PERST# is asserted. So I definitely agree that it makes sense for all EPF function drivers to do both alloc_space() + set_bar() is a single call, now when that is possible. This will simplify a lot of PCIe endpoint code considerably. Kind regards, Niklas
