diff mbox series

[v5,2/6] PCI: dwc: ep: Add missing checks when dynamically changing a BAR

Message ID 20241127103016.3481128-10-cassel@kernel.org (mailing list archive)
State New
Headers show
Series PCI endpoint additional pci_epc_set_bar() checks | expand

Commit Message

Niklas Cassel Nov. 27, 2024, 10:30 a.m. UTC
In commit 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update
inbound map address") set_bar() was modified to support dynamically
changing the physical address of a BAR.

This means that set_bar() can be called twice, without ever calling
clear_bar(), as calling clear_bar() would clear the BAR's PCI address
assigned by the host).

This can only be done if the new BAR configuration doesn't fundamentally
differ from the existing BAR configuration. Add these missing checks.

While at it, add comments which clarifies the support for dynamically
changing the physical address of a BAR. (Which was also missing.)

Fixes: 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address")
Signed-off-by: Niklas Cassel <cassel@kernel.org>
---
 .../pci/controller/dwc/pcie-designware-ep.c   | 22 ++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

Comments

Manivannan Sadhasivam Nov. 30, 2024, 8:24 a.m. UTC | #1
On Wed, Nov 27, 2024 at 11:30:18AM +0100, Niklas Cassel wrote:
> In commit 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update
> inbound map address") set_bar() was modified to support dynamically
> changing the physical address of a BAR.
> 
> This means that set_bar() can be called twice, without ever calling
> clear_bar(), as calling clear_bar() would clear the BAR's PCI address
> assigned by the host).
> 
> This can only be done if the new BAR configuration doesn't fundamentally
> differ from the existing BAR configuration. Add these missing checks.
> 
> While at it, add comments which clarifies the support for dynamically
> changing the physical address of a BAR. (Which was also missing.)
> 
> Fixes: 4284c88fff0e ("PCI: designware-ep: Allow pci_epc_set_bar() update inbound map address")
> Signed-off-by: Niklas Cassel <cassel@kernel.org>

Reviewed-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>

- Mani

> ---
>  .../pci/controller/dwc/pcie-designware-ep.c   | 22 ++++++++++++++++++-
>  1 file changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index bad588ef69a4..01c739aaf61a 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -222,8 +222,28 @@ static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
>  	if ((flags & PCI_BASE_ADDRESS_MEM_TYPE_64) && (bar & 1))
>  		return -EINVAL;
>  
> -	if (ep->epf_bar[bar])
> +	/*
> +	 * Certain EPF drivers dynamically change the physical address of a BAR
> +	 * (i.e. they call set_bar() twice, without ever calling clear_bar(), as
> +	 * calling clear_bar() would clear the BAR's PCI address assigned by the
> +	 * host).
> +	 */
> +	if (ep->epf_bar[bar]) {
> +		/*
> +		 * We can only dynamically change a BAR if the new configuration
> +		 * doesn't fundamentally differ from the existing configuration.
> +		 */
> +		if (ep->epf_bar[bar]->barno != bar ||
> +		    ep->epf_bar[bar]->size != size ||
> +		    ep->epf_bar[bar]->flags != flags)
> +			return -EINVAL;
> +
> +		/*
> +		 * When dynamically changing a BAR, skip writing the BAR reg, as
> +		 * that would clear the BAR's PCI address assigned by the host.
> +		 */
>  		goto config_atu;
> +	}
>  
>  	reg = PCI_BASE_ADDRESS_0 + (4 * bar);
>  
> -- 
> 2.47.0
>
diff mbox series

Patch

diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
index bad588ef69a4..01c739aaf61a 100644
--- a/drivers/pci/controller/dwc/pcie-designware-ep.c
+++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
@@ -222,8 +222,28 @@  static int dw_pcie_ep_set_bar(struct pci_epc *epc, u8 func_no, u8 vfunc_no,
 	if ((flags & PCI_BASE_ADDRESS_MEM_TYPE_64) && (bar & 1))
 		return -EINVAL;
 
-	if (ep->epf_bar[bar])
+	/*
+	 * Certain EPF drivers dynamically change the physical address of a BAR
+	 * (i.e. they call set_bar() twice, without ever calling clear_bar(), as
+	 * calling clear_bar() would clear the BAR's PCI address assigned by the
+	 * host).
+	 */
+	if (ep->epf_bar[bar]) {
+		/*
+		 * We can only dynamically change a BAR if the new configuration
+		 * doesn't fundamentally differ from the existing configuration.
+		 */
+		if (ep->epf_bar[bar]->barno != bar ||
+		    ep->epf_bar[bar]->size != size ||
+		    ep->epf_bar[bar]->flags != flags)
+			return -EINVAL;
+
+		/*
+		 * When dynamically changing a BAR, skip writing the BAR reg, as
+		 * that would clear the BAR's PCI address assigned by the host.
+		 */
 		goto config_atu;
+	}
 
 	reg = PCI_BASE_ADDRESS_0 + (4 * bar);