linux-kernel - Re: [PATCH] PCI: qcom: Implement shutdown() callback

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 for Android: free password hash cracker in your pocket

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

Message-ID: <20250423171715.GA430351@bhelgaas>
Date: Wed, 23 Apr 2025 12:17:15 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	linux-pci@...r.kernel.org, linux-arm-msm@...r.kernel.org,
	linux-kernel@...r.kernel.org, quic_mrana@...cinc.com,
	quic_vbadigan@...cinc.com, quic_ramkri@...cinc.com,
	quic_vpernami@...cinc.com,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Danilo Krummrich <dakr@...nel.org>,
	Heiner Kallweit <hkallweit1@...il.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	netdev@...r.kernel.org, Niklas Cassel <cassel@...nel.org>
Subject: Re: [PATCH] PCI: qcom: Implement shutdown() callback

[+cc Greg, Rafael, Danilo for driver model .shutdown() question]
[+cc Heiner et al for related conversation at
https://lore.kernel.org/r/20250415095335.506266-2-cassel@kernel.org]

On Tue, Apr 01, 2025 at 04:51:37PM +0530, Krishna Chaitanya Chundru wrote:
> From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> 
> PCIe host controller drivers are supposed to properly remove the
> endpoint drivers and release the resources during host shutdown/reboot
> to avoid issues like smmu errors, NOC errors, etc.

The effect of this patch is:

    .shutdown()
  +   qcom_pcie_shutdown
  +     dw_pcie_host_deinit
  +       pci_stop_root_bus     # release all drivers of downstream pci_devs
  +       pci_remove_root_bus   # remove all downstream pci_devs

I'm not sure about removing all these drivers in the .shutdown() path.
The generic .shutdown() doc is "quiesce the device" [1], and my
current interpretation for PCI is that it should disable DMA and
interrupts from the device [2].

If PCI host controller drivers are supposed to remove all downstream
drivers and devices in .shutdown(), they're all broken because that's
currently only done in .remove() (and not even all of those).

[1] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/device/driver.h?id=v6.14#n73
[2] https://lore.kernel.org/all/61f70fd6-52fd-da07-ce73-303f95132131@codeaurora.org/

> So, stop and remove the root bus and its associated devices and release
> its resources during system shutdown to ensure a clean shutdown/reboot.
> 
> Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> Signed-off-by: Krishna Chaitanya Chundru <krishna.chundru@....qualcomm.com>
> ---
>  drivers/pci/controller/dwc/pcie-qcom.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> index e4d3366ead1f9198693e6f9da4ae1dc40a3a0519..926811a0e63eb3663c1f41dc598659993546d832 100644
> --- a/drivers/pci/controller/dwc/pcie-qcom.c
> +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> @@ -1754,6 +1754,16 @@ static int qcom_pcie_probe(struct platform_device *pdev)
>  	return ret;
>  }
>  
> +static void qcom_pcie_shutdown(struct platform_device *pdev)
> +{
> +	struct qcom_pcie *pcie = platform_get_drvdata(pdev);
> +
> +	dw_pcie_host_deinit(&pcie->pci->pp);
> +	phy_exit(pcie->phy);
> +	pm_runtime_put(&pdev->dev);
> +	pm_runtime_disable(&pdev->dev);
> +}
> +
>  static int qcom_pcie_suspend_noirq(struct device *dev)
>  {
>  	struct qcom_pcie *pcie = dev_get_drvdata(dev);
> @@ -1890,5 +1900,6 @@ static struct platform_driver qcom_pcie_driver = {
>  		.pm = &qcom_pcie_pm_ops,
>  		.probe_type = PROBE_PREFER_ASYNCHRONOUS,
>  	},
> +	.shutdown = qcom_pcie_shutdown,
>  };
>  builtin_platform_driver(qcom_pcie_driver);