Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1398231 > unrolled thread

[PATCH 00/11] Various IRQ and GIC fixes and clean-ups

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/11] Various IRQ and GIC fixes and clean-ups Jon Hunter <jonathanh@nvidia.com> - 2016-05-10 17:30 +0200
    [PATCH 04/11] irqchip/gic: Don't unnecessarily write the IRQ configuration Jon Hunter <jonathanh@nvidia.com> - 2016-05-10 17:30 +0200
    [PATCH 01/11] genirq: Ensure IRQ descriptor is valid when setting-up the IRQ Jon Hunter <jonathanh@nvidia.com> - 2016-05-10 17:30 +0200
    [PATCH 05/11] irqchip/gic: WARN if setting the interrupt type for a PPI fails Jon Hunter <jonathanh@nvidia.com> - 2016-05-10 17:30 +0200

#1398231 — [PATCH 00/11] Various IRQ and GIC fixes and clean-ups

Most of these patches were originally part of the series to add support
for the Tegra210 AGIC [0]. However, given that series has grown and some
of the changes to the IRQ core needed some more review/testing, per an
offline discussion with Marc Z, I have split out the more trivial fixes
and clean-up patches from the core changes.

[0] http://marc.info/?l=linux-tegra&m=146237957525813&w=2

Jon Hunter (11):
  genirq: Ensure IRQ descriptor is valid when setting-up the IRQ
  irqdomain: Warn if we fail to set the IRQ type
  irqchip: Mask the non-type/sense bits when translating an IRQ
  irqchip/gic: Don't unnecessarily write the IRQ configuration
  irqchip/gic: WARN if setting the interrupt type for a PPI fails
  irqchip/gic: Don't initialise chip if mapping IO space fails
  irqchip/gic: Remove static irq_chip definition for eoimode1
  irqchip/gic: Return an error if GIC initialisation fails
  irqchip/gic: Pass GIC pointer to save/restore functions
  irqchip/gic: Store GIC configuration parameters
  irqchip/gic: Add helper functions for GIC setup and teardown

 drivers/irqchip/irq-crossbar.c   |   2 +-
 drivers/irqchip/irq-gic-common.c |  20 ++-
 drivers/irqchip/irq-gic.c        | 315 +++++++++++++++++++++++++--------------
 drivers/irqchip/irq-tegra.c      |   2 +-
 kernel/irq/irqdomain.c           |   3 +-
 kernel/irq/manage.c              |   2 +-
 6 files changed, 221 insertions(+), 123 deletions(-)

-- 
2.1.4

[toc] | [next] | [standalone]

#1398234 — [PATCH 04/11] irqchip/gic: Don't unnecessarily write the IRQ configuration

If the interrupt configuration matches the current configuration, then
don't bother writing the configuration again.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-common.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index 2e9443be2b14..eeeefa244933 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -63,13 +63,17 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 	else if (type & IRQ_TYPE_EDGE_BOTH)
 		val |= confmask;
 
+	/* If the current configuration is the same, then we are done */
+	if (val == oldval)
+		return 0;
+
 	/*
 	 * Write back the new configuration, and possibly re-enable
-	 * the interrupt. If we tried to write a new configuration and failed,
+	 * the interrupt. If we fail to write a new configuration,
 	 * return an error.
 	 */
 	writel_relaxed(val, base + GIC_DIST_CONFIG + confoff);
-	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val && val != oldval)
+	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val)
 		ret = -EINVAL;
 
 	if (sync_access)
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1398241 — [PATCH 01/11] genirq: Ensure IRQ descriptor is valid when setting-up the IRQ

In the function, setup_irq(), we don't check that the descriptor
returned from irq_to_desc() is valid before we start using it. For
example chip_bus_lock() called from setup_irq(), assumes that the
descriptor pointer is valid and doesn't check before dereferencing it.

In many other functions including setup/free_percpu_irq() we do check
that the descriptor returned is not NULL and therefore add the same test
to setup_irq() to ensure the descriptor returned is valid.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
---
 kernel/irq/manage.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index cc1cc641d653..ef0bc02c3a70 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -1407,7 +1407,7 @@ int setup_irq(unsigned int irq, struct irqaction *act)
 	int retval;
 	struct irq_desc *desc = irq_to_desc(irq);
 
-	if (WARN_ON(irq_settings_is_per_cpu_devid(desc)))
+	if (!desc || WARN_ON(irq_settings_is_per_cpu_devid(desc)))
 		return -EINVAL;
 	chip_bus_lock(desc);
 	retval = __setup_irq(irq, desc, act);
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1398246 — [PATCH 05/11] irqchip/gic: WARN if setting the interrupt type for a PPI fails

Setting the interrupt type for private peripheral interrupts (PPIs) may
not be supported by a given GIC because it is IMPLEMENTATION DEFINED
whether this is allowed. There is no way to know if setting the type is
supported for a given GIC and so the value written is read back to
verify it matches the desired configuration. If it does not match then
an error is return.

There are cases where the interrupt configuration read from firmware
(such as a device-tree blob), has been incorrect and hence
gic_configure_irq() has returned an error. This error has gone
undetected because the error code returned was ignored but the interrupt
still worked fine because the configuration for the interrupt could not
be overwritten.

Given that this has done undetected and that failing to set the
configuration for a PPI may not be a catastrophic, don't return an error
but WARN if we fail to configure a PPI. This will allows us to fix up
any places in the kernel where we should be checking the return status
and maintain backward compatibility with firmware images that may have
incorrect PPI configurations.

Signed-off-by: Jon Hunter <jonathanh@nvidia.com>
Acked-by: Marc Zyngier <marc.zyngier@arm.com>
---
 drivers/irqchip/irq-gic-common.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c
index eeeefa244933..89e7423f0ebb 100644
--- a/drivers/irqchip/irq-gic-common.c
+++ b/drivers/irqchip/irq-gic-common.c
@@ -69,12 +69,20 @@ int gic_configure_irq(unsigned int irq, unsigned int type,
 
 	/*
 	 * Write back the new configuration, and possibly re-enable
-	 * the interrupt. If we fail to write a new configuration,
-	 * return an error.
+	 * the interrupt. If we fail to write a new configuration for
+	 * an SPI then WARN and return an error. If we fail to write the
+	 * configuration for a PPI this is most likely because the GIC
+	 * does not allow us to set the configuration or we are in a
+	 * non-secure mode, and hence it may not be catastrophic.
 	 */
 	writel_relaxed(val, base + GIC_DIST_CONFIG + confoff);
-	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val)
-		ret = -EINVAL;
+	if (readl_relaxed(base + GIC_DIST_CONFIG + confoff) != val) {
+		if (WARN_ON(irq >= 32))
+			ret = -EINVAL;
+		else
+			pr_warn("GIC: PPI%d is secure or misconfigured\n",
+				irq - 16);
+	}
 
 	if (sync_access)
 		sync_access();
-- 
2.1.4

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web