| 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
| ||
|
Message-ID: <20240716102002.GL3446@thinkpad>
Date: Tue, 16 Jul 2024 15:50:02 +0530
From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
To: neil.armstrong@...aro.org
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>,
Krzysztof Wilczyński <kw@...ux.com>,
Rob Herring <robh@...nel.org>, Bjorn Helgaas <bhelgaas@...gle.com>,
Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>,
Kishon Vijay Abraham I <kishon@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>, linux-pci@...r.kernel.org,
linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
devicetree@...r.kernel.org
Subject: Re: [PATCH 13/14] PCI: qcom: Simulate PCIe hotplug using 'global'
interrupt
On Tue, Jul 16, 2024 at 10:40:55AM +0200, neil.armstrong@...aro.org wrote:
> On 15/07/2024 19:33, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> >
> > Historically, Qcom PCIe RC controllers lack standard hotplug support. So
> > when an endpoint is attached to the SoC, users have to rescan the bus
> > manually to enumerate the device. But this can be avoided by simulating the
> > PCIe hotplug using Qcom specific way.
> >
> > Qcom PCIe RC controllers are capable of generating the 'global' SPI
> > interrupt to the host CPUs. The device driver can use this event to
> > identify events such as PCIe link specific events, safety events etc...
> >
> > One such event is the PCIe Link up event generated when an endpoint is
> > detected on the bus and the Link is 'up'. This event can be used to
> > simulate the PCIe hotplug in the Qcom SoCs.
> >
> > So add support for capturing the PCIe Link up event using the 'global'
> > interrupt in the driver. Once the Link up event is received, the bus
> > underneath the host bridge is scanned to enumerate PCIe endpoint devices,
> > thus simulating hotplug.
> >
> > All of the Qcom SoCs have only one rootport per controller instance. So
> > only a single 'Link up' event is generated for the PCIe controller.
> >
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > ---
> > drivers/pci/controller/dwc/pcie-qcom.c | 55 ++++++++++++++++++++++++++++++++++
> > 1 file changed, 55 insertions(+)
> >
> > diff --git a/drivers/pci/controller/dwc/pcie-qcom.c b/drivers/pci/controller/dwc/pcie-qcom.c
> > index 0180edf3310e..38ed411d2052 100644
> > --- a/drivers/pci/controller/dwc/pcie-qcom.c
> > +++ b/drivers/pci/controller/dwc/pcie-qcom.c
> > @@ -50,6 +50,9 @@
> > #define PARF_AXI_MSTR_WR_ADDR_HALT_V2 0x1a8
> > #define PARF_Q2A_FLUSH 0x1ac
> > #define PARF_LTSSM 0x1b0
> > +#define PARF_INT_ALL_STATUS 0x224
> > +#define PARF_INT_ALL_CLEAR 0x228
> > +#define PARF_INT_ALL_MASK 0x22c
> > #define PARF_SID_OFFSET 0x234
> > #define PARF_BDF_TRANSLATE_CFG 0x24c
> > #define PARF_SLV_ADDR_SPACE_SIZE 0x358
> > @@ -121,6 +124,9 @@
> > /* PARF_LTSSM register fields */
> > #define LTSSM_EN BIT(8)
> > +/* PARF_INT_ALL_{STATUS/CLEAR/MASK} register fields */
> > +#define PARF_INT_ALL_LINK_UP BIT(13)
> > +
> > /* PARF_NO_SNOOP_OVERIDE register fields */
> > #define WR_NO_SNOOP_OVERIDE_EN BIT(1)
> > #define RD_NO_SNOOP_OVERIDE_EN BIT(3)
> > @@ -260,6 +266,7 @@ struct qcom_pcie {
> > struct icc_path *icc_cpu;
> > const struct qcom_pcie_cfg *cfg;
> > struct dentry *debugfs;
> > + int global_irq;
>
> I think you can drop this, the irq number is no more needed after probe
>
> > bool suspended;
> > };
> > @@ -1488,6 +1495,29 @@ static void qcom_pcie_init_debugfs(struct qcom_pcie *pcie)
> > qcom_pcie_link_transition_count);
> > }
> > +static irqreturn_t qcom_pcie_global_irq_thread(int irq, void *data)
> > +{
> > + struct qcom_pcie *pcie = data;
> > + struct dw_pcie_rp *pp = &pcie->pci->pp;
> > + struct device *dev = pcie->pci->dev;
> > + u32 status = readl_relaxed(pcie->parf + PARF_INT_ALL_STATUS);
> > +
> > + writel_relaxed(status, pcie->parf + PARF_INT_ALL_CLEAR);
> > +
> > + if (FIELD_GET(PARF_INT_ALL_LINK_UP, status)) {
> > + dev_dbg(dev, "Received Link up event. Starting enumeration!\n");
> > + /* Rescan the bus to enumerate endpoint devices */
> > + pci_lock_rescan_remove();
> > + pci_rescan_bus(pp->bridge->bus);
> > + pci_unlock_rescan_remove();
> > + } else {
> > + dev_err(dev, "Received unknown event. INT_STATUS: 0x%08x\n",
> > + status);
>
> Can this happen ? perhaps dev_warn_once instead ?
>
I did see one such issue that went unreported and ended by taking some debug
cycles. But dev_warn_once() makes sense.
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > static int qcom_pcie_probe(struct platform_device *pdev)
> > {
> > const struct qcom_pcie_cfg *pcie_cfg;
> > @@ -1498,6 +1528,7 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > struct dw_pcie_rp *pp;
> > struct resource *res;
> > struct dw_pcie *pci;
> > + char *name;
> > int ret;
> > pcie_cfg = of_device_get_match_data(dev);
> > @@ -1617,6 +1648,28 @@ static int qcom_pcie_probe(struct platform_device *pdev)
> > goto err_phy_exit;
> > }
> > + name = devm_kasprintf(dev, GFP_KERNEL, "qcom_pcie_global_irq%d",
> > + pci_domain_nr(pp->bridge->bus));
> > + if (!name) {
> > + ret = -ENOMEM;
> > + goto err_host_deinit;
> > + }
> > +
> > + pcie->global_irq = platform_get_irq_byname_optional(pdev, "global");
> > + if (pcie->global_irq > 0) {
> > + ret = devm_request_threaded_irq(&pdev->dev, pcie->global_irq,
> > + NULL,
> > + qcom_pcie_global_irq_thread,
> > + IRQF_ONESHOT, name, pcie);
> > + if (ret) {
> > + dev_err_probe(&pdev->dev, ret,
> > + "Failed to request Global IRQ\n");
> > + goto err_host_deinit;
> > + }
> > +
> > + writel_relaxed(PARF_INT_ALL_LINK_UP, pcie->parf + PARF_INT_ALL_MASK);
>
> Is this available on all PCIe RC core versions ?
> perhaps this should be moved into a callback of ops_1_9_0 for now ?
>
This register should be available on all version afaik. Even if it is not on
some legacy ones, it can be moved later if required (once the dts for those
platforms start defining 'global' interrupt).
- Mani
--
மணிவண்ணன் சதாசிவம்
Powered by blists - more mailing lists