Re: [PATCH v4 1/4] PCI: Move pci_enable_acs() and its dependencies up in pci.c

[Bjorn Helgaas]

On Tue, Jul 07, 2020 at 03:46:01PM -0700, Rajat Jain wrote:
> Move pci_enable_acs() and the functions it depends on, further up in the
> source code to avoid having to forward declare it when we make it static
> in near future (next patch).
> 
> No functional changes intended.
> 
> Signed-off-by: Rajat Jain <rajatja@xxxxxxxxxx>

Applied patches 1-3 to pci/enumeration for v5.9, thanks!

I held off on patch 4 (enabling PCI_ACS_TB) until we have a little
more conversation on the impact of it.

> ---
> v4: Same as v3
> v3: Initial version of the patch, created per Bjorn's suggestion
> 
>  drivers/pci/pci.c | 254 +++++++++++++++++++++++-----------------------
>  1 file changed, 127 insertions(+), 127 deletions(-)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index ce096272f52b1..eec625f0e594e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -777,6 +777,133 @@ int pci_wait_for_pending(struct pci_dev *dev, int pos, u16 mask)
>  	return 0;
>  }
>  
> +static int pci_acs_enable;
> +
> +/**
> + * pci_request_acs - ask for ACS to be enabled if supported
> + */
> +void pci_request_acs(void)
> +{
> +	pci_acs_enable = 1;
> +}
> +
> +static const char *disable_acs_redir_param;
> +
> +/**
> + * pci_disable_acs_redir - disable ACS redirect capabilities
> + * @dev: the PCI device
> + *
> + * For only devices specified in the disable_acs_redir parameter.
> + */
> +static void pci_disable_acs_redir(struct pci_dev *dev)
> +{
> +	int ret = 0;
> +	const char *p;
> +	int pos;
> +	u16 ctrl;
> +
> +	if (!disable_acs_redir_param)
> +		return;
> +
> +	p = disable_acs_redir_param;
> +	while (*p) {
> +		ret = pci_dev_str_match(dev, p, &p);
> +		if (ret < 0) {
> +			pr_info_once("PCI: Can't parse disable_acs_redir parameter: %s\n",
> +				     disable_acs_redir_param);
> +
> +			break;
> +		} else if (ret == 1) {
> +			/* Found a match */
> +			break;
> +		}
> +
> +		if (*p != ';' && *p != ',') {
> +			/* End of param or invalid format */
> +			break;
> +		}
> +		p++;
> +	}
> +
> +	if (ret != 1)
> +		return;
> +
> +	if (!pci_dev_specific_disable_acs_redir(dev))
> +		return;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> +	if (!pos) {
> +		pci_warn(dev, "cannot disable ACS redirect for this hardware as it does not have ACS capabilities\n");
> +		return;
> +	}
> +
> +	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> +
> +	/* P2P Request & Completion Redirect */
> +	ctrl &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC);
> +
> +	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> +
> +	pci_info(dev, "disabled ACS redirect\n");
> +}
> +
> +/**
> + * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
> + * @dev: the PCI device
> + */
> +static void pci_std_enable_acs(struct pci_dev *dev)
> +{
> +	int pos;
> +	u16 cap;
> +	u16 ctrl;
> +
> +	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> +	if (!pos)
> +		return;
> +
> +	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
> +	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> +
> +	/* Source Validation */
> +	ctrl |= (cap & PCI_ACS_SV);
> +
> +	/* P2P Request Redirect */
> +	ctrl |= (cap & PCI_ACS_RR);
> +
> +	/* P2P Completion Redirect */
> +	ctrl |= (cap & PCI_ACS_CR);
> +
> +	/* Upstream Forwarding */
> +	ctrl |= (cap & PCI_ACS_UF);
> +
> +	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> +}
> +
> +/**
> + * pci_enable_acs - enable ACS if hardware support it
> + * @dev: the PCI device
> + */
> +void pci_enable_acs(struct pci_dev *dev)
> +{
> +	if (!pci_acs_enable)
> +		goto disable_acs_redir;
> +
> +	if (!pci_dev_specific_enable_acs(dev))
> +		goto disable_acs_redir;
> +
> +	pci_std_enable_acs(dev);
> +
> +disable_acs_redir:
> +	/*
> +	 * Note: pci_disable_acs_redir() must be called even if ACS was not
> +	 * enabled by the kernel because it may have been enabled by
> +	 * platform firmware.  So if we are told to disable it, we should
> +	 * always disable it after setting the kernel's default
> +	 * preferences.
> +	 */
> +	pci_disable_acs_redir(dev);
> +}
> +
>  /**
>   * pci_restore_bars - restore a device's BAR values (e.g. after wake-up)
>   * @dev: PCI device to have its BARs restored
> @@ -3230,133 +3357,6 @@ void pci_configure_ari(struct pci_dev *dev)
>  	}
>  }
>  
> -static int pci_acs_enable;
> -
> -/**
> - * pci_request_acs - ask for ACS to be enabled if supported
> - */
> -void pci_request_acs(void)
> -{
> -	pci_acs_enable = 1;
> -}
> -
> -static const char *disable_acs_redir_param;
> -
> -/**
> - * pci_disable_acs_redir - disable ACS redirect capabilities
> - * @dev: the PCI device
> - *
> - * For only devices specified in the disable_acs_redir parameter.
> - */
> -static void pci_disable_acs_redir(struct pci_dev *dev)
> -{
> -	int ret = 0;
> -	const char *p;
> -	int pos;
> -	u16 ctrl;
> -
> -	if (!disable_acs_redir_param)
> -		return;
> -
> -	p = disable_acs_redir_param;
> -	while (*p) {
> -		ret = pci_dev_str_match(dev, p, &p);
> -		if (ret < 0) {
> -			pr_info_once("PCI: Can't parse disable_acs_redir parameter: %s\n",
> -				     disable_acs_redir_param);
> -
> -			break;
> -		} else if (ret == 1) {
> -			/* Found a match */
> -			break;
> -		}
> -
> -		if (*p != ';' && *p != ',') {
> -			/* End of param or invalid format */
> -			break;
> -		}
> -		p++;
> -	}
> -
> -	if (ret != 1)
> -		return;
> -
> -	if (!pci_dev_specific_disable_acs_redir(dev))
> -		return;
> -
> -	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> -	if (!pos) {
> -		pci_warn(dev, "cannot disable ACS redirect for this hardware as it does not have ACS capabilities\n");
> -		return;
> -	}
> -
> -	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> -
> -	/* P2P Request & Completion Redirect */
> -	ctrl &= ~(PCI_ACS_RR | PCI_ACS_CR | PCI_ACS_EC);
> -
> -	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> -
> -	pci_info(dev, "disabled ACS redirect\n");
> -}
> -
> -/**
> - * pci_std_enable_acs - enable ACS on devices using standard ACS capabilities
> - * @dev: the PCI device
> - */
> -static void pci_std_enable_acs(struct pci_dev *dev)
> -{
> -	int pos;
> -	u16 cap;
> -	u16 ctrl;
> -
> -	pos = pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ACS);
> -	if (!pos)
> -		return;
> -
> -	pci_read_config_word(dev, pos + PCI_ACS_CAP, &cap);
> -	pci_read_config_word(dev, pos + PCI_ACS_CTRL, &ctrl);
> -
> -	/* Source Validation */
> -	ctrl |= (cap & PCI_ACS_SV);
> -
> -	/* P2P Request Redirect */
> -	ctrl |= (cap & PCI_ACS_RR);
> -
> -	/* P2P Completion Redirect */
> -	ctrl |= (cap & PCI_ACS_CR);
> -
> -	/* Upstream Forwarding */
> -	ctrl |= (cap & PCI_ACS_UF);
> -
> -	pci_write_config_word(dev, pos + PCI_ACS_CTRL, ctrl);
> -}
> -
> -/**
> - * pci_enable_acs - enable ACS if hardware support it
> - * @dev: the PCI device
> - */
> -void pci_enable_acs(struct pci_dev *dev)
> -{
> -	if (!pci_acs_enable)
> -		goto disable_acs_redir;
> -
> -	if (!pci_dev_specific_enable_acs(dev))
> -		goto disable_acs_redir;
> -
> -	pci_std_enable_acs(dev);
> -
> -disable_acs_redir:
> -	/*
> -	 * Note: pci_disable_acs_redir() must be called even if ACS was not
> -	 * enabled by the kernel because it may have been enabled by
> -	 * platform firmware.  So if we are told to disable it, we should
> -	 * always disable it after setting the kernel's default
> -	 * preferences.
> -	 */
> -	pci_disable_acs_redir(dev);
> -}
> -
>  static bool pci_acs_flags_enabled(struct pci_dev *pdev, u16 acs_flags)
>  {
>  	int pos;
> -- 
> 2.27.0.212.ge8ba1cc988-goog
>