Hello Frank,
On Wed, Nov 20, 2024 at 12:37:17PM -0500, Frank Li wrote:
> On Tue, Nov 19, 2024 at 10:44:38AM +0100, Niklas Cassel wrote:
> >
> > Frank, since you are the author of:
> > 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address")
> >
> > Please advice on what to do here. The only thing that makes sense to me is
> > that dw_pcie_ep_set_bar() always writes the BAR MASK. If dw_pcie_ep_set_bar()
> > does NOT write the BAR MASK, we depend on whatever BAR MASK was there before
> > dw_pcie_ep_set_bar(), which no matter how I look at it, seems horribly wrong.
>
> dw_pcie_ep_set_bar() don't change BAR MASK because host side also use it
> to allocate resource when probe. But it just change iATU map address, for
> example 1M BAR1, map to EP's side 0x1000_0000, it may change to another
> address 0x2000_0000. In doorbell MSI case, you tested, it dynamtic change
> to ITS address, then change back when do normal bar test, which easiest
> way to understand the behavior.
>
> I think the order write iATU and mask doesn't matter because hardware
> should just ignores some bits according to mask register.
Well, since LWR_TARGET_RW and LWR_TARGET_HW field sizes depends on the
value in BAR_MASK, of course the order matters.
BAR_MASK will always have some value, and if we write the iATU registers
before writing the BAR_MASK, then we will be at the mercy of the reset
value of the BAR_MASK register.
I sent a fix for this here:
https://lore.kernel.org/linux-pci/20241122115709.2949703-8-cassel@xxxxxxxxxx/
I now understand why you did the early return.
I cleaned up the code, and added some comments explaining how the support
for dynamically changing the physical address of a BAR works.
It would be nice if you could test so that your vNTB use case still works.
Kind regards,
Niklas
