From: Frank Li <Frank.li@nxp.com>
To: Niklas Cassel <Niklas.Cassel@wdc.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>,
Vidya Sagar <vidyas@nvidia.com>,
"helgaas@kernel.org" <helgaas@kernel.org>,
"kishon@ti.com" <kishon@ti.com>,
"lorenzo.pieralisi@arm.com" <lorenzo.pieralisi@arm.com>,
"kw@linux.com" <kw@linux.com>,
"jingoohan1@gmail.com" <jingoohan1@gmail.com>,
"gustavo.pimentel@synopsys.com" <gustavo.pimentel@synopsys.com>,
"lznuaa@gmail.com" <lznuaa@gmail.com>,
"hongxing.zhu@nxp.com" <hongxing.zhu@nxp.com>,
"jdmason@kudzu.us" <jdmason@kudzu.us>,
"dave.jiang@intel.com" <dave.jiang@intel.com>,
"allenbh@gmail.com" <allenbh@gmail.com>,
"linux-ntb@googlegroups.com" <linux-ntb@googlegroups.com>,
"linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>
Subject: Re: [PATCH v2 1/4] PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address
Date: Thu, 14 Dec 2023 15:52:43 -0500 [thread overview]
Message-ID: <ZXtrG40SR81YAR6a@lizhi-Precision-Tower-5810> (raw)
In-Reply-To: <ZXtkMC1ZjsgHMRvT@x1-carbon>
On Thu, Dec 14, 2023 at 08:23:14PM +0000, Niklas Cassel wrote:
> On Thu, Dec 14, 2023 at 10:19:03AM -0500, Frank Li wrote:
> > On Thu, Dec 14, 2023 at 02:31:04PM +0000, Niklas Cassel wrote:
> > > Hello Frank,
> > >
> > > On Tue, Feb 22, 2022 at 10:23:52AM -0600, Frank Li wrote:
> > > > ntb_mw_set_trans() will set memory map window after endpoint function
> > > > driver bind. The inbound map address need be updated dynamically when
> > > > using NTB by PCIe Root Port and PCIe Endpoint connection.
> > > >
> > > > Checking if iatu already assigned to the BAR, if yes, using assigned iatu
> > > > number to update inbound address map and skip set BAR's register.
> > > >
> > > > Signed-off-by: Frank Li <Frank.Li@nxp.com>
> > > > ---
> > > >
> > > > Change from V1:
> > > > - improve commit message
> > > >
> > > > drivers/pci/controller/dwc/pcie-designware-ep.c | 10 +++++++++-
> > > > 1 file changed, 9 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > index 998b698f40858..cad5d9ea7cc6c 100644
> > > > --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> > > > @@ -161,7 +161,11 @@ static int dw_pcie_ep_inbound_atu(struct dw_pcie_ep *ep, u8 func_no,
> > > > u32 free_win;
> > > > struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> > > >
> > > > - free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
> > >
> > > find_first_zero_bit() can return 0, representing bit 0,
> > > which is a perfectly valid return value.
> > >
> > > > + if (!ep->bar_to_atu[bar])
> > >
> > > so this check is not correct.
> > >
> >
> > Please sent out your fixed patch. If want to me fix it, please tell me
> > reproduce steps.
>
> Reproduce steps are simple:
> 1) Add debug messages to dw_pcie_ep_inbound_atu() to see the iATU index for
> each BAR.
> 2) Boot an EP platform with a core_init_notifier.
> 3) Boot the RC.
> 4) Reboot the RC, which will assert + deassert PERST, and will call
> pci_epc_init_notify(), which will call .core_init (pci_epf_test_core_init())
> which will set the BARs.
>
>
> In step 3) BAR0 will use iATU0.
>
> In step 4) BAR0 will use iATU6 instead of iATU0.
> There is no reason for this, as it should really reuse the same
> iATU index as before, just like all the other BARs do.
> (This is because of find_first_zero_bit() misusage.)
>
>
> I could send out my patch, but from inspecting the code, it looks like:
>
> > > > + if (ep->epf_bar[bar])
> > > > + return 0;
I checked current code.
dw_pcie_ep_inbound_atu()
{
if (!ep->bar_to_atu[bar])
free_win = find_first_zero_bit(ep->ib_window_map, pci->num_ib_windows);
else
free_win = ep->bar_to_atu[bar];
}
I missed conside '0' is validate windows number. I think we should init
bar_to_atu to -1.
if (ep->bar_to_atu[bar] < 0)
Origial change want dw_pcie_ep_inbound_atu() can be call twice to update
bar map address. vntb use "bar3" as memory map windows, so have not trigger
this problem.
Frank
>
> from dw_pcie_ep_set_bar(), also needs to be dropped, so that the iATU
> settings will be re-written for platforms with core_init_notifiers.
>
> Right now, for a platform with a core_init_notifier, if you run
> pci_endpoint_test + reboot the RC (so that PERST is asserted + deasserted),
> and then run pci_endpoint_test again, then I'm quite sure that
> pci_endpoint_test will not pass the second time (because iATU settings
> were not rewritten after reset).
>
> It would be nice if Mani or Vidya could confirm this.
>
>
> I guess that you added this statement for some reason, so I assume
> that we can't just drop this line without breaking something else.
>
> I guess one option would be modify dw_pcie_ep_init_notify() to call
> dw_pcie_ep_clear_bar() on all non-NULL BARs stored in ep->epf_bar[],
> before calling pci_epc_init_notify(). That way the second .core_init
> (pci_epf_test_core_init()) call will use write the settings, because
> ep->epf_bar[] will be empty, so the "write the settings only the first
> time" approach will then also work for core_init_notifier platforms.
>
>
> Kind regards,
> Niklas
next prev parent reply other threads:[~2023-12-14 20:52 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-02-22 16:23 [PATCH V2 0/4] NTB function for PCIe RC to EP connection Frank Li
2022-02-22 16:23 ` [PATCH v2 1/4] PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address Frank Li
2023-12-14 14:31 ` Niklas Cassel
2023-12-14 15:19 ` Frank Li
2023-12-14 20:23 ` Niklas Cassel
2023-12-14 20:52 ` Frank Li [this message]
2023-12-14 21:28 ` Frank Li
2023-12-16 10:11 ` Niklas Cassel
2023-12-19 17:59 ` Manivannan Sadhasivam
2023-12-20 5:14 ` Damien Le Moal
2023-12-20 6:03 ` Manivannan Sadhasivam
2023-12-20 7:06 ` Damien Le Moal
2023-12-14 21:39 ` Niklas Cassel
2022-02-22 16:23 ` [PATCH v2 2/4] NTB: epf: Allow more flexibility in the memory BAR map method Frank Li
2022-03-10 22:08 ` Zhi Li
2022-02-22 16:23 ` [PATCH v2 3/4] PCI: endpoint: Support NTB transfer between RC and EP Frank Li
2022-03-10 22:09 ` Zhi Li
2022-12-14 0:08 ` Bjorn Helgaas
[not found] ` <CAHrpEqSGySHDET3YPu3czzoMBmCRJsgGgU4s3GWWbtruFLVHaA@mail.gmail.com>
2022-12-14 0:28 ` Bjorn Helgaas
2022-12-14 0:47 ` [EXT] " Frank Li
2022-02-22 16:23 ` [PATCH v2 4/4] Documentation: PCI: Add specification for the PCI vNTB function device Frank Li
2022-03-10 22:01 ` [PATCH V2 0/4] NTB function for PCIe RC to EP connection Zhi Li
2022-03-10 22:07 ` Zhi Li
2022-04-04 20:12 ` Zhi Li
2022-04-05 10:34 ` Kishon Vijay Abraham I
2022-04-05 15:35 ` Zhi Li
2022-04-20 20:22 ` Zhi Li
2022-04-22 15:15 ` Kishon Vijay Abraham I
2022-04-22 15:36 ` Zhi Li
2022-08-12 14:02 ` Jon Mason
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZXtrG40SR81YAR6a@lizhi-Precision-Tower-5810 \
--to=frank.li@nxp.com \
--cc=Niklas.Cassel@wdc.com \
--cc=allenbh@gmail.com \
--cc=dave.jiang@intel.com \
--cc=gustavo.pimentel@synopsys.com \
--cc=helgaas@kernel.org \
--cc=hongxing.zhu@nxp.com \
--cc=jdmason@kudzu.us \
--cc=jingoohan1@gmail.com \
--cc=kishon@ti.com \
--cc=kw@linux.com \
--cc=linux-ntb@googlegroups.com \
--cc=linux-pci@vger.kernel.org \
--cc=lorenzo.pieralisi@arm.com \
--cc=lznuaa@gmail.com \
--cc=manivannan.sadhasivam@linaro.org \
--cc=vidyas@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).