[PATCH] PCI: endpoint: pci-epf-test: Fix doorbell test support

[PATCH] PCI: endpoint: pci-epf-test: Fix doorbell test support

[Niklas Cassel]

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@nxp.com>
Fixes: eff0c286aa91 ("PCI: endpoint: pci-epf-test: Add doorbell test support")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
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@nxp.com/
However, the code was modified, without notifying the list of this
non-trivial logical change, before being applied.

 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

Re: [PATCH] PCI: endpoint: pci-epf-test: Fix doorbell test support

[Frank Li]

On Mon, Sep 08, 2025 at 06:19:42PM +0200, 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@nxp.com>
> Fixes: eff0c286aa91 ("PCI: endpoint: pci-epf-test: Add doorbell test support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Frank Li <Frank.Li@nxp.com>

> ---
> 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@nxp.com/
> However, the code was modified, without notifying the list of this
> non-trivial logical change, before being applied.
>
>  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
>

Re: [PATCH] PCI: endpoint: pci-epf-test: Fix doorbell test support

[Manivannan Sadhasivam]

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@nxp.com>
> Fixes: eff0c286aa91 ("PCI: endpoint: pci-epf-test: Add doorbell test support")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>
> ---
> 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@nxp.com/
> 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
> 

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH] PCI: endpoint: pci-epf-test: Fix doorbell test support

[Manivannan Sadhasivam]

On Mon, 08 Sep 2025 18:19:42 +0200, 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.
> 
> [...]

Applied, thanks!

[1/1] PCI: endpoint: pci-epf-test: Fix doorbell test support
      commit: 54a8c34df746c979037c907ad7fe42c52a58b532

Best regards,
-- 
Manivannan Sadhasivam <mani@kernel.org>