linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: Marc Zyngier <maz@kernel.org>
To: linux-tegra@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org
Cc: Venkat Reddy Talla <vreddytalla@nvidia.com>,
	Jonathan Hunter <jonathanh@nvidia.com>,
	Thierry Reding <thierry.reding@gmail.com>,
	Sowjanya Komatineni <skomatineni@nvidia.com>,
	Dmitry Osipenko <digetx@gmail.com>,
	kernel-team@android.com, Thomas Gleixner <tglx@linutronix.de>
Subject: [PATCH 3/3] soc/tegra: pmc: Don't create fake interrupt hierarchy levels
Date: Mon,  5 Oct 2020 12:14:43 +0100	[thread overview]
Message-ID: <20201005111443.1390096-4-maz@kernel.org> (raw)
In-Reply-To: <20201005111443.1390096-1-maz@kernel.org>

The Tegra PMC driver does ungodly things with the interrupt hierarchy,
repeatedly corrupting it by pulling hwirq numbers out of thin air,
overriding existing IRQ mappings and changing the handling flow
of unsuspecting users.

All of this is done in the name of preserving the interrupt hierarchy
even when these levels do not exist in the HW. Together with the use
of proper IRQs for IPIs, this leads to an unbootable system as the
rescheduling IPI gets repeatedly repurposed for random drivers...

Instead, let's allow the hierarchy to be trimmed to the level that
actually makes sense for the HW, and not any deeper. This avoids
having unnecessary callbacks, overriding mappings, and otherwise
keeps the hierarchy sane.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 drivers/soc/tegra/pmc.c | 79 +++++++++++++++--------------------------
 1 file changed, 29 insertions(+), 50 deletions(-)

diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index 9960f7c18431..4eea3134fb3e 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -1993,6 +1993,30 @@ static int tegra_pmc_irq_translate(struct irq_domain *domain,
 	return 0;
 }
 
+/* Trim the irq hierarchy from a particular irq domain */
+static void trim_hierarchy(unsigned int virq, struct irq_domain *domain)
+{
+	struct irq_data *tail, *irq_data = irq_get_irq_data(virq);
+
+	/* The PMC doesn't generate any interrupt by itself */
+	if (WARN_ON(!irq_data->parent_data))
+		return;
+
+	/* Skip until we find the right domain */
+	while (irq_data->parent_data && irq_data->parent_data->domain != domain)
+		irq_data = irq_data->parent_data;
+
+	/* Sever the inner part of the hierarchy...  */
+	tail = irq_data->parent_data;
+	irq_data->parent_data = NULL;
+
+	/* ... and free it */
+	for (irq_data = tail; irq_data; irq_data = tail) {
+		tail = irq_data->parent_data;
+		kfree(irq_data);
+	};
+}
+
 static int tegra_pmc_irq_alloc(struct irq_domain *domain, unsigned int virq,
 			       unsigned int num_irqs, void *data)
 {
@@ -2039,46 +2063,15 @@ static int tegra_pmc_irq_alloc(struct irq_domain *domain, unsigned int virq,
 
 			err = irq_domain_set_hwirq_and_chip(domain, virq,
 							    event->id,
-							    &pmc->irq, pmc);
-
-			/*
-			 * GPIOs don't have an equivalent interrupt in the
-			 * parent controller (GIC). However some code, such
-			 * as the one in irq_get_irqchip_state(), require a
-			 * valid IRQ chip to be set. Make sure that's the
-			 * case by passing NULL here, which will install a
-			 * dummy IRQ chip for the interrupt in the parent
-			 * domain.
-			 */
-			if (domain->parent)
-				irq_domain_set_hwirq_and_chip(domain->parent,
-							      virq, 0, NULL,
-							      NULL);
-
+							    &pmc_irqchip, pmc);
+			if (!err)
+				trim_hierarchy(virq, domain->parent);
 			break;
 		}
 	}
 
-	/*
-	 * For interrupts that don't have associated wake events, assign a
-	 * dummy hardware IRQ number. This is used in the ->irq_set_type()
-	 * and ->irq_set_wake() callbacks to return early for these IRQs.
-	 */
-	if (i == soc->num_wake_events) {
-		err = irq_domain_set_hwirq_and_chip(domain, virq, ULONG_MAX,
-						    &pmc_irqchip, pmc);
-
-		/*
-		 * Interrupts without a wake event don't have a corresponding
-		 * interrupt in the parent controller (GIC). Pass NULL for the
-		 * chip here, which causes a dummy IRQ chip to be installed
-		 * for the interrupt in the parent domain, to make this
-		 * explicit.
-		 */
-		if (domain->parent)
-			irq_domain_set_hwirq_and_chip(domain->parent, virq, 0,
-						      NULL, NULL);
-	}
+	if (i == soc->num_wake_events)
+		trim_hierarchy(virq, domain);
 
 	return err;
 }
@@ -2094,9 +2087,6 @@ static int tegra210_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
 	unsigned int offset, bit;
 	u32 value;
 
-	if (data->hwirq == ULONG_MAX)
-		return 0;
-
 	offset = data->hwirq / 32;
 	bit = data->hwirq % 32;
 
@@ -2131,9 +2121,6 @@ static int tegra210_pmc_irq_set_type(struct irq_data *data, unsigned int type)
 	unsigned int offset, bit;
 	u32 value;
 
-	if (data->hwirq == ULONG_MAX)
-		return 0;
-
 	offset = data->hwirq / 32;
 	bit = data->hwirq % 32;
 
@@ -2174,10 +2161,6 @@ static int tegra186_pmc_irq_set_wake(struct irq_data *data, unsigned int on)
 	unsigned int offset, bit;
 	u32 value;
 
-	/* nothing to do if there's no associated wake event */
-	if (WARN_ON(data->hwirq == ULONG_MAX))
-		return 0;
-
 	offset = data->hwirq / 32;
 	bit = data->hwirq % 32;
 
@@ -2205,10 +2188,6 @@ static int tegra186_pmc_irq_set_type(struct irq_data *data, unsigned int type)
 	struct tegra_pmc *pmc = irq_data_get_irq_chip_data(data);
 	u32 value;
 
-	/* nothing to do if there's no associated wake event */
-	if (data->hwirq == ULONG_MAX)
-		return 0;
-
 	value = readl(pmc->wake + WAKE_AOWAKE_CNTRL(data->hwirq));
 
 	switch (type) {
-- 
2.28.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  parent reply	other threads:[~2020-10-05 11:16 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-05 11:14 [PATCH 0/3] soc/tegra: Prevent the PMC driver from corrupting interrupt routing Marc Zyngier
2020-10-05 11:14 ` [PATCH 1/3] gpio: tegra186: Allow optional irq parent callbacks Marc Zyngier
2020-10-05 11:14 ` [PATCH 2/3] soc/tegra: pmc: " Marc Zyngier
2020-10-05 11:27   ` Thierry Reding
2020-10-05 12:59     ` Marc Zyngier
2020-10-05 11:14 ` Marc Zyngier [this message]
2020-10-05 11:33   ` [PATCH 3/3] soc/tegra: pmc: Don't create fake interrupt hierarchy levels Thierry Reding
2020-10-05 13:10     ` Marc Zyngier
2020-10-05 11:22 ` [PATCH 0/3] soc/tegra: Prevent the PMC driver from corrupting interrupt routing Thierry Reding
2020-10-05 13:06   ` Marc Zyngier
2020-10-05 15:45     ` Thierry Reding
2020-10-05 18:23       ` Jon Hunter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201005111443.1390096-4-maz@kernel.org \
    --to=maz@kernel.org \
    --cc=digetx@gmail.com \
    --cc=jonathanh@nvidia.com \
    --cc=kernel-team@android.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=skomatineni@nvidia.com \
    --cc=tglx@linutronix.de \
    --cc=thierry.reding@gmail.com \
    --cc=vreddytalla@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).