linux-kernel - Re: [PATCH 2/8] PCI/bwctrl: Add support to scale bandwidth before & after link re-training

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f58ff91f-95a1-1a0d-91e9-972f0eeddd4c@linux.intel.com>
Date: Mon, 17 Feb 2025 11:28:49 +0200 (EET)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
cc: Bjorn Helgaas <bhelgaas@...gle.com>, Jingoo Han <jingoohan1@...il.com>, 
    Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>, 
    Lorenzo Pieralisi <lpieralisi@...nel.org>, 
    Krzysztof Wilczyński <kw@...ux.com>, 
    Rob Herring <robh@...nel.org>, Johannes Berg <johannes@...solutions.net>, 
    Jeff Johnson <jjohnson@...nel.org>, linux-pci@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>, linux-arm-msm@...r.kernel.org, 
    mhi@...ts.linux.dev, linux-wireless@...r.kernel.org, 
    ath11k@...ts.infradead.org, quic_jjohnson@...cinc.com, 
    quic_pyarlaga@...cinc.com, quic_vbadigan@...cinc.com, 
    quic_vpernami@...cinc.com, quic_mrana@...cinc.com
Subject: Re: [PATCH 2/8] PCI/bwctrl: Add support to scale bandwidth before
 & after link re-training

On Mon, 17 Feb 2025, Krishna Chaitanya Chundru wrote:

> If the driver wants to move to higher data rate/speed than the current data
> rate then the controller driver may need to change certain votes so that
> link may come up at requested data rate/speed like QCOM PCIe controllers
> need to change their RPMh (Resource Power Manager-hardened) state. Once
> link retraining is done controller drivers needs to adjust their votes
> based on the final data rate.
> 
> Some controllers also may need to update their bandwidth voting like
> ICC bw votings etc.
> 
> So, add pre_scale_bus_bw() & post_scale_bus_bw() op to call before & after
> the link re-train.
> 
> In case of PCIe switch, if there is a request to change target speed for a
> downstream port then no need to call these function ops as these are
> outside the scope of the controller drivers.
> 
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> ---
>  drivers/pci/pcie/bwctrl.c | 15 +++++++++++++++
>  include/linux/pci.h       |  2 ++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/pci/pcie/bwctrl.c b/drivers/pci/pcie/bwctrl.c
> index 0a5e7efbce2c..e3faa4d1f935 100644
> --- a/drivers/pci/pcie/bwctrl.c
> +++ b/drivers/pci/pcie/bwctrl.c
> @@ -161,6 +161,8 @@ static int pcie_bwctrl_change_speed(struct pci_dev *port, u16 target_speed, bool
>  int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req,
>  			  bool use_lt)
>  {
> +	struct pci_host_bridge *host = pci_find_host_bridge(port->bus);
> +	bool is_root = pci_is_root_bus(port->bus);

is_rootport ?

>  	struct pci_bus *bus = port->subordinate;
>  	u16 target_speed;
>  	int ret;
> @@ -173,6 +175,16 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req,
>  
>  	target_speed = pcie_bwctrl_select_speed(port, speed_req);
>  
> +	/*
> +	 * The controller driver may need to be scaled for targeted speed
> +	 * otherwise link might not come up at requested speed.
> +	 */
> +	if (is_root && host->ops->pre_scale_bus_bw) {
> +		ret = host->ops->pre_scale_bus_bw(host->bus, target_speed);
> +		if (ret)
> +			return ret;
> +	}
> +
>  	scoped_guard(rwsem_read, &pcie_bwctrl_setspeed_rwsem) {
>  		struct pcie_bwctrl_data *data = port->link_bwctrl;
>  
> @@ -197,6 +209,9 @@ int pcie_set_target_speed(struct pci_dev *port, enum pci_bus_speed speed_req,
>  	    !list_empty(&bus->devices))
>  		ret = -EAGAIN;
>  
> +	if (is_root && host->ops->post_scale_bus_bw)
> +		host->ops->post_scale_bus_bw(host->bus, pci_bus_speed2lnkctl2(bus->cur_bus_speed));

Is the naming of these callbacks too specific for your use case? Does PCIe 
spec actually call changing the Target Speed "scaling bus bandwidth" or 
something along those line?

>  	return ret;
>  }
>  
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 47b31ad724fa..58f1de626c37 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -804,6 +804,8 @@ struct pci_ops {
>  	void __iomem *(*map_bus)(struct pci_bus *bus, unsigned int devfn, int where);
>  	int (*read)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *val);
>  	int (*write)(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 val);
> +	int (*pre_scale_bus_bw)(struct pci_bus *bus, int target_speed);
> +	void (*post_scale_bus_bw)(struct pci_bus *bus, int current_speed);

Please document these, including the locking requirements.

-- 
 i.