On Fri, Jan 23, 2026 at 09:51:19AM +0100, Niklas Cassel wrote: > On Fri, Jan 23, 2026 at 10:16:21AM +0900, Koichiro Den wrote: > > > > > > There might be other EPC drivers that don't disable all BARs in their > > > .init(), so I would say that simply checking if the BAR has an address is > > > not sufficient to guarantee that an EPF driver has called set_bar(). > > > > > > > Even if an EPC driver does not reset the BAR in their .init() and some > > default translation is left exposed, wouldn't it be safe as long as > > dw_pcie_ep_ib_atu_addr() succeeds in programming inbound mappings for the > > entire BAR? > > For e.g. on RK3588, the default HW configuration of the DWC controller has > all 5 BARs as enabled, with a size of 1 GB. > > There is no inbound address translation for these BARs by default. > > So for it to be safe, the size of the set_bar() call would have to > match the current size of the BAR, but how should the EPF driver know > that when it has not called set_bar() yet? > > dw_pcie_ep_read_bar_assigned() does not return the current size of the > BAR. So you can't verify that the set_bar() call has the same size as > the BARs "default size".
I wasn't considering either of the following cases as unsafe: - succeeding by chance in programming via a one-shot set_bar() with submaps - such a set_bar() failing (due to incorrect size recognition) while as I mentioned in my previous reply, the first case effectively becomes a loophole that contradicts the docs and git commit messages. However, since v8, the second case clears any existing mappings, which could indeed lead to an unsafe situtation. > > > > > > That said, such usage apparently contradicts the documented usage (1st > > set_bar with no submap, then with submap) described in the docs and commit > > messages in this series, and allowing it would make things unnecessarily > > complicated. So I agree that adding such a safeguard is the right approach. > > > > > > > > I think the safest option is my second suggestion because then we know > > > that we will only call > > > dw_pcie_ep_ib_atu_addr() > > > > > > When: > > > > > > 1) If ep->epf_bar[bar] is set: > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L363 > > > > > > > > > 2) All the other requirements to dynamically update a BAR is also met: > > > > > > https://github.com/torvalds/linux/blob/v6.19-rc6/drivers/pci/controller/dwc/pcie-designware-ep.c#L368-L370 > > > > > > > That makes sense, and it ensures that the behavior always accords with the > > docs and commit messages in this series. > > I think it makes most sense to put the "use_addr_translation = true" > > after the check: > > /* > * We can only dynamically change a BAR if the new BAR size and > * BAR flags do not differ from the existing configuration. > */ > if (ep->epf_bar[bar]->barno != bar || > ep->epf_bar[bar]->size != size || > ep->epf_bar[bar]->flags != flags) > return -EINVAL; > > > So we know that dw_pcie_ep_ib_atu_addr() is only called when the size is the > same. I'll send v10 with the fix, possibly adding a BAR_SUBRANGE_TEST to pci endpoint test as well. Kind regards, Koichiro > > > Kind regards, > Niklas
