linux-kernel - Re: [PATCH v12 8/8] PCI: endpoint: Remove "core_init

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: <20240412202216.GA14590@bhelgaas>
Date: Fri, 12 Apr 2024 15:22:16 -0500
From: Bjorn Helgaas <helgaas@...nel.org>
To: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: Jingoo Han <jingoohan1@...il.com>,
	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
	Marek Vasut <marek.vasut+renesas@...il.com>,
	Yoshihiro Shimoda <yoshihiro.shimoda.uh@...esas.com>,
	Thierry Reding <thierry.reding@...il.com>,
	Jonathan Hunter <jonathanh@...dia.com>,
	Vidya Sagar <vidyas@...dia.com>,
	Vignesh Raghavendra <vigneshr@...com>,
	Richard Zhu <hongxing.zhu@....com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	NXP Linux Team <linux-imx@....com>,
	Minghuan Lian <minghuan.Lian@....com>,
	Mingkai Hu <mingkai.hu@....com>, Roy Zang <roy.zang@....com>,
	Kunihiko Hayashi <hayashi.kunihiko@...ionext.com>,
	Masami Hiramatsu <mhiramat@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>,
	Jesper Nilsson <jesper.nilsson@...s.com>,
	Srikanth Thokala <srikanth.thokala@...el.com>,
	Shawn Lin <shawn.lin@...k-chips.com>,
	Heiko Stuebner <heiko@...ech.de>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-renesas-soc@...r.kernel.org,
	linux-arm-msm@...r.kernel.org, linux-tegra@...r.kernel.org,
	linux-omap@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	linuxppc-dev@...ts.ozlabs.org, Niklas Cassel <cassel@...nel.org>,
	linux-arm-kernel@...s.com, linux-rockchip@...ts.infradead.org
Subject: Re: [PATCH v12 8/8] PCI: endpoint: Remove "core_init_notifier" flag

On Wed, Mar 27, 2024 at 02:43:37PM +0530, Manivannan Sadhasivam wrote:
> "core_init_notifier" flag is set by the glue drivers requiring refclk from
> the host to complete the DWC core initialization. Also, those drivers will
> send a notification to the EPF drivers once the initialization is fully
> completed using the pci_epc_init_notify() API. Only then, the EPF drivers
> will start functioning.
> 
> For the rest of the drivers generating refclk locally, EPF drivers will
> start functioning post binding with them. EPF drivers rely on the
> 'core_init_notifier' flag to differentiate between the drivers.
> Unfortunately, this creates two different flows for the EPF drivers.
> 
> So to avoid that, let's get rid of the "core_init_notifier" flag and follow
> a single initialization flow for the EPF drivers. This is done by calling
> the dw_pcie_ep_init_notify() from all glue drivers after the completion of
> dw_pcie_ep_init_registers() API. This will allow all the glue drivers to
> send the notification to the EPF drivers once the initialization is fully
> completed.

Thanks for doing this!  I think this is a significantly nicer
solution than core_init_notifier was.

One question: both qcom and tegra194 call dw_pcie_ep_init_registers()
from an interrupt handler, but they register that handler in a
different order with respect to dw_pcie_ep_init().

I don't know what actually starts the process that leads to the
interrupt, but if it's dw_pcie_ep_init(), then one of these (qcom, I
think) must be racy:

  qcom_pcie_ep_probe
    dw_pcie_ep_init                                             <- A
    qcom_pcie_ep_enable_irq_resources
      devm_request_threaded_irq(qcom_pcie_ep_perst_irq_thread)  <- B

  qcom_pcie_ep_perst_irq_thread
    qcom_pcie_perst_deassert
      dw_pcie_ep_init_registers

  tegra_pcie_dw_probe
    tegra_pcie_config_ep
      devm_request_threaded_irq(tegra_pcie_ep_pex_rst_irq)      <- B
      dw_pcie_ep_init                                           <- A

  tegra_pcie_ep_pex_rst_irq
    pex_ep_event_pex_rst_deassert
      dw_pcie_ep_init_registers

Whatever the right answer is, I think qcom and tegra194 should both
order dw_pcie_ep_init() and the devm_request_threaded_irq() the same
way.

Bjorn