On Mon, Sep 08, 2025 at 06:19:42PM GMT, Niklas Cassel wrote:
> The doorbell feature temporarily overrides the inbound translation to
> point to the address stored in epf_test->db_bar.phys_addr.
> (I.e. it calls set_bar() twice, without ever calling clear_bar(), as
> calling clear_bar() would clear the BAR's PCI address assigned by the
> host).
>
> Thus, when disabling the doorbell, restore the inbound translation to
> point to the memory allocated for the BAR.
>
> Without this, running the pci endpoint kselftest doorbell test case more
> than once would fail.
>
> Cc: Frank Li <Frank.Li@xxxxxxx>
> Fixes: eff0c286aa91 ("PCI: endpoint: pci-epf-test: Add doorbell test support")
> Signed-off-by: Niklas Cassel <cassel@xxxxxxxxxx>
> ---
> Note: this is actually how the code looked like when it was submitted by
> Frank, see pci_epf_test_disable_doorbell() in:
> https://lore.kernel.org/linux-pci/20250710-ep-msi-v21-6-57683fc7fb25@xxxxxxx/
> However, the code was modified, without notifying the list of this
> non-trivial logical change, before being applied.
>
Yes, it was my mistake. I did some cleanup while applying, but since I didn't
had access to the board, I couldn't test it. Anyhow, I should've notified the
thread.
- Mani
> drivers/pci/endpoint/functions/pci-epf-test.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/endpoint/functions/pci-epf-test.c b/drivers/pci/endpoint/functions/pci-epf-test.c
> index e091193bd8a8a..b6ca1766a4ca9 100644
> --- a/drivers/pci/endpoint/functions/pci-epf-test.c
> +++ b/drivers/pci/endpoint/functions/pci-epf-test.c
> @@ -772,12 +772,24 @@ static void pci_epf_test_disable_doorbell(struct pci_epf_test *epf_test,
> u32 status = le32_to_cpu(reg->status);
> struct pci_epf *epf = epf_test->epf;
> struct pci_epc *epc = epf->epc;
> + int ret;
>
> if (bar < BAR_0)
> goto set_status_err;
>
> pci_epf_test_doorbell_cleanup(epf_test);
> - pci_epc_clear_bar(epc, epf->func_no, epf->vfunc_no, &epf_test->db_bar);
> +
> + /*
> + * The doorbell feature temporarily overrides the inbound translation to
> + * point to the address stored in epf_test->db_bar.phys_addr.
> + * (I.e. it calls set_bar() twice, without ever calling clear_bar(), as
> + * calling clear_bar() would clear the BAR's PCI address assigned by the
> + * host). Thus, when disabling the doorbell, restore the inbound
> + * translation to point to the memory allocated for the BAR.
> + */
> + ret = pci_epc_set_bar(epc, epf->func_no, epf->vfunc_no, &epf->bar[bar]);
> + if (ret)
> + goto set_status_err;
>
> status |= STATUS_DOORBELL_DISABLE_SUCCESS;
> reg->status = cpu_to_le32(status);
> --
> 2.51.0
>
--
மணிவண்ணன் சதாசிவம்
