Hello Hayashisan, On Mon, Dec 16, 2024 at 04:39:41PM +0900, Kunihiko Hayashi wrote: > There are bar numbers that cannot be used on the endpoint. > So instead of SoC-specific conditions, add "reserved_bar" bar number > bitmap to the SoC data. I think that it was mistake to put is_am654_pci_dev() checks in pci_endpoint_test.c in the first place. However, let's not make the situation worse by introducing a reserved_bar bitmap on the host side as well. IMO, we should not have any logic for this the host side at all. Just like for am654, rk3588 has a BAR (BAR4) that should not be written by pci_endpoint_test.c (as it exposes the ATU registers in BAR4, so if the host writes this BAR, all address translation will be broken). An EPC driver can mark a BAR as reserved and that is exactly was rk3588 does for BAR4: https://github.com/torvalds/linux/blob/v6.13-rc3/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L300 Marking a BAR as reserved means that an EPF driver should not touch that BAR at all. However, this by itself is not enough if the BAR is enabled by default, in that case we also need to disable the BAR for the host side to not be able to write to it: https://github.com/torvalds/linux/blob/v6.13-rc3/drivers/pci/controller/dwc/pcie-dw-rockchip.c#L248-L249 If we look at am654, we can see that it does set BAR0 and BAR1 as reserved: https://github.com/torvalds/linux/blob/v6.13-rc3/drivers/pci/controller/dwc/pci-keystone.c#L967-L968 The problem is that am654 does not also disable these BARs by default. If you look at most DWC based EPC drivers: drivers/pci/controller/dwc/pci-dra7xx.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pci-imx6.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pci-layerscape-ep.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-artpec6.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-designware-plat.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-dw-rockchip.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-qcom-ep.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-rcar-gen4.c: dw_pcie_ep_reset_bar(pci, bar); drivers/pci/controller/dwc/pcie-uniphier-ep.c: dw_pcie_ep_reset_bar(pci, bar); They call dw_pcie_ep_reset_bar() in EP init for all BARs. (An EPF driver will be able to re-enable all BARs that are not marked as reserved.) am654 seems to be the exception here. If you simply add code that disables all BARs by default in am654, you should be able to remove these ugly is_am654_pci_dev() checks in the host driver, and the host driver should not be able to write to these reserved BARs, as they will never get enabled by pci-epf-test.c. Kind regards, Niklas
