Devicetree
 help / color / mirror / Atom feed
* [PATCH v2 0/2] Add Xilinx Versal NET SMMU CSR interrupt controller support
@ 2026-09-18  6:39 Tushar Nimkar
  2026-09-18  6:39 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add Xilinx Versal NET SMMU CSR Tushar Nimkar
  2026-09-18  6:39 ` [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver Tushar Nimkar
  0 siblings, 2 replies; 5+ messages in thread
From: Tushar Nimkar @ 2026-09-18  6:39 UTC (permalink / raw)
  To: Anirudha Sarangi, Thomas Gleixner, Radu Rendec, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michal Simek
  Cc: git, linux-arm-kernel, linux-kernel, devicetree, Tushar Nimkar

On Xilinx Versal NET, the ARM SMMUv3 EVENTQ, PRIQ and GERROR interrupts
are routed through a vendor-specific Control and Status Register (CSR)
block, where they must be enabled, cleared and acknowledged before being
forwarded to the parent GIC.
This series adds a small irqchip driver for the CSR block that chains to
the standard arm-smmu-v3 driver, so the generic SMMUv3 driver needs no
changes.
Patch 1 adds the DT binding.
Patch 2 adds the irqchip driver.

Changes in v2:
- Posted as a proper threaded series with a cover letter (v1 patches
  went out individually).
- dt-bindings (Krzysztof):
  - Dropped the irrelevant arm,smmu-v3 consumer node from the example.
  - Dropped the unused "smmu_irq_csr" label from the example.
  - Dropped the redundant "binding" from the patch subject.
- irqchip driver (Thomas):
  - Aligned kernel-doc member descriptions and used "interrupt"
    instead of "IRQ".
  - Used guard(raw_spinlock)() in the mask/unmask/ack callbacks and
    addressed the comments related to lock handling.
  - Thomas suggested using devm-managed allocation and irqdomain APIs;
    however, the driver is registered via IRQCHIP_DECLARE() and has no
    associated struct device, so devm APIs cannot be used. This code is
    left unchanged.

Link to v1:
https://lore.kernel.org/all/20260817105233.1557748-1-tushar.nimkar@amd.com/
https://lore.kernel.org/all/20260817105251.1557770-1-tushar.nimkar@amd.com/

Signed-off-by: Tushar Nimkar <tushar.nimkar@amd.com>
---
Anirudha Sarangi (2):
      dt-bindings: interrupt-controller: Add Xilinx Versal NET SMMU CSR
      irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver

 .../xlnx,versal-net-smmu-csr.yaml                  |  75 +++++++
 MAINTAINERS                                        |  10 +
 drivers/irqchip/Kconfig                            |  10 +
 drivers/irqchip/Makefile                           |   1 +
 drivers/irqchip/irq-xilinx-smmu-csr.c              | 216 +++++++++++++++++++++
 5 files changed, 312 insertions(+)
---
base-commit: 0d9d0dbf2fddcff5859d623e90ca73c4054276e1
change-id: 20260917-smmu-changes-v2-9cf7435d6969

Best regards,
--  
Tushar Nimkar <tushar.nimkar@amd.com>


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 1/2] dt-bindings: interrupt-controller: Add Xilinx Versal NET SMMU CSR
  2026-09-18  6:39 [PATCH v2 0/2] Add Xilinx Versal NET SMMU CSR interrupt controller support Tushar Nimkar
@ 2026-09-18  6:39 ` Tushar Nimkar
  2026-09-18  6:39 ` [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver Tushar Nimkar
  1 sibling, 0 replies; 5+ messages in thread
From: Tushar Nimkar @ 2026-09-18  6:39 UTC (permalink / raw)
  To: Anirudha Sarangi, Thomas Gleixner, Radu Rendec, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michal Simek
  Cc: git, linux-arm-kernel, linux-kernel, devicetree, Tushar Nimkar

From: Anirudha Sarangi <anirudha.sarangi@amd.com>

Add a device tree binding schema for the Xilinx Versal NET SMMU CSR
(Control and Status Register) interrupt controller.

The SMMU CSR block acts as a vendor-specific interrupt controller
in front of the ARM SMMUv3, providing registers to enable and clear
standard SMMU interrupts such as EVENTQ, CMDQ_SYNC, GERROR and PRIQ.
Interrupts must be acknowledged in this block before being forwarded
to the parent interrupt controller (e.g. GIC).

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@amd.com>
Co-developed-by: Tushar Nimkar <tushar.nimkar@amd.com>
Signed-off-by: Tushar Nimkar <tushar.nimkar@amd.com>
---
 .../xlnx,versal-net-smmu-csr.yaml                  | 75 ++++++++++++++++++++++
 1 file changed, 75 insertions(+)

diff --git a/Documentation/devicetree/bindings/interrupt-controller/xlnx,versal-net-smmu-csr.yaml b/Documentation/devicetree/bindings/interrupt-controller/xlnx,versal-net-smmu-csr.yaml
new file mode 100644
index 000000000000..38b63e51b732
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/xlnx,versal-net-smmu-csr.yaml
@@ -0,0 +1,75 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/interrupt-controller/xlnx,versal-net-smmu-csr.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Xilinx Versal NET SMMU CSR Interrupt Controller
+
+maintainers:
+  - Anirudha Sarangi <anirudha.sarangi@amd.com>
+  - Tushar Nimkar <tushar.nimkar@amd.com>
+
+description: |
+  The Versal NET platform includes a Xilinx-specific SMMU CSR (Control and
+  Status Register) block that acts as an intermediate interrupt controller
+  for ARM SMMUv3.
+
+  The block provides registers to enable, disable and clear the
+  standard SMMUv3 interrupt sources (EVENTQ, CMDQ_SYNC, GERROR and PRIQ)
+  before forwarding them to the parent interrupt controller,
+  typically a GIC.
+
+  The controller receives one parent interrupt and demultiplexes CSR
+  status bits into the following child interrupt identifiers
+    0 - EVENTQ
+    1 - CMDQ_SYNC
+    2 - GERROR (GLOBAL)
+    3 - PRIQ
+
+allOf:
+  - $ref: /schemas/interrupt-controller.yaml#
+
+properties:
+  compatible:
+    const: xlnx,versal-net-smmu-csr
+
+  reg:
+    maxItems: 1
+
+  interrupt-controller: true
+
+  "#interrupt-cells":
+    const: 1
+    description: |
+     Single cell containing the child interrupt identifier
+      0 - EVENTQ
+      1 - CMDQ_SYNC
+      2 - GERROR (GLOBAL)
+      3 - PRIQ
+
+  interrupts:
+    maxItems: 1
+
+required:
+  - compatible
+  - reg
+  - interrupt-controller
+  - "#interrupt-cells"
+  - interrupts
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    interrupt-controller@eca10000 {
+        compatible = "xlnx,versal-net-smmu-csr";
+        reg = <0xeca10000 0x1000>;
+        interrupt-controller;
+        #interrupt-cells = <1>;
+        interrupt-parent = <&gic>;
+        interrupts = <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>;
+    };

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver
  2026-09-18  6:39 [PATCH v2 0/2] Add Xilinx Versal NET SMMU CSR interrupt controller support Tushar Nimkar
  2026-09-18  6:39 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add Xilinx Versal NET SMMU CSR Tushar Nimkar
@ 2026-09-18  6:39 ` Tushar Nimkar
  2026-09-18  6:48   ` sashiko-bot
  1 sibling, 1 reply; 5+ messages in thread
From: Tushar Nimkar @ 2026-09-18  6:39 UTC (permalink / raw)
  To: Anirudha Sarangi, Thomas Gleixner, Radu Rendec, Rob Herring,
	Krzysztof Kozlowski, Conor Dooley, Michal Simek
  Cc: git, linux-arm-kernel, linux-kernel, devicetree, Tushar Nimkar

From: Anirudha Sarangi <anirudha.sarangi@amd.com>

The Xilinx Versal NET SoC implements a Control and Status Register (CSR)
block in front of the ARM SMMUv3 to manage EVENTQ, PRIQ and GLOBAL
interrupts. Interrupts are enabled, cleared, and acknowledged
through this block by writing to the relevant registers. Once an
interrupt is acknowledged, it is forwarded to the parent interrupt
controller for further processing.

Add an irqchip driver for the CSR block. The driver registers as an
interrupt controller and chains to the upstream SMMUv3 driver, so
that the standard SMMUv3 driver does not require modification while still
supporting Xilinx SoCs.

The CSR block can also gate the SMMUv3 CMDQ_SYNC completion interrupt
(bit 1), but it is intentionally left unsupported. The Linux arm-smmu-v3
driver signals CMDQ_SYNC completion via polling or MSI and never requests
a wired CMD_SYNC interrupt, so there is no in-kernel consumer for it.
Exposing it as a child line would only risk spurious, never-acked
interrupts, and it is omitted from the DT binding for the same reason.

This driver is intended to operate in conjunction with the ARM SMMUv3
driver and the kernel's IOMMU DMA infrastructure. These dependencies
are not selected directly in Kconfig to avoid introducing dependency
recursion.

Signed-off-by: Anirudha Sarangi <anirudha.sarangi@amd.com>
Co-developed-by: Tushar Nimkar <tushar.nimkar@amd.com>
Signed-off-by: Tushar Nimkar <tushar.nimkar@amd.com>
---
 MAINTAINERS                           |  10 ++
 drivers/irqchip/Kconfig               |  10 ++
 drivers/irqchip/Makefile              |   1 +
 drivers/irqchip/irq-xilinx-smmu-csr.c | 216 ++++++++++++++++++++++++++++++++++
 4 files changed, 237 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 6e50747a621c..40ef350285a6 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -30141,6 +30141,16 @@ F:	Documentation/misc-devices/xilinx_sdfec.rst
 F:	drivers/misc/xilinx_sdfec.c
 F:	include/uapi/misc/xilinx_sdfec.h
 
+XILINX SMMU CSR IRQ DRIVER
+M:	Anirudha Sarangi <anirudha.sarangi@amd.com>
+M:	Tushar Nimkar <tushar.nimkar@amd.com>
+L:	git@amd.com
+L:	linux-arm-kernel@lists.infradead.org
+L:	linux-kernel@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/interrupt-controller/xlnx,versal-net-smmu-csr.yaml
+F:	drivers/irqchip/irq-xilinx-smmu-csr.c
+
 XILINX TRNG DRIVER
 M:	Mounika Botcha <mounika.botcha@amd.com>
 M:	Harsh Jain <h.jain@amd.com>
diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig
index a8f8c423b795..81a2c9107601 100644
--- a/drivers/irqchip/Kconfig
+++ b/drivers/irqchip/Kconfig
@@ -842,4 +842,14 @@ config SUNPLUS_SP7021_INTC
 	  chained controller, routing all interrupt source in P-Chip to
 	  the primary controller on C-Chip.
 
+config IRQCHIP_XILINX_SMMU_CSR
+	bool "Xilinx SMMU CSR Interrupt Driver"
+	depends on ARCH_ZYNQMP
+	select IRQ_DOMAIN
+	help
+	  This driver does the initial handling of SMMU interrupts before
+	  forwarding them to the ARM SMMU v3 driver.  It is intended for
+	  ZynqMP-class platforms where a vendor CSR block mediates SMMU
+	  interrupts.
+
 endmenu
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index ab33cccd8471..16a5fd126c9a 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -135,3 +135,4 @@ obj-$(CONFIG_APPLE_AIC)			+= irq-apple-aic.o
 obj-$(CONFIG_MCHP_EIC)			+= irq-mchp-eic.o
 obj-$(CONFIG_SOPHGO_SG2042_MSI)		+= irq-sg2042-msi.o
 obj-$(CONFIG_SUNPLUS_SP7021_INTC)	+= irq-sp7021-intc.o
+obj-$(CONFIG_IRQCHIP_XILINX_SMMU_CSR)	+= irq-xilinx-smmu-csr.o
diff --git a/drivers/irqchip/irq-xilinx-smmu-csr.c b/drivers/irqchip/irq-xilinx-smmu-csr.c
new file mode 100644
index 000000000000..d9b575c6c58b
--- /dev/null
+++ b/drivers/irqchip/irq-xilinx-smmu-csr.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Xilinx Versal Net SMMU CSR interrupt controller driver
+ *
+ * Copyright (C) 2026 Advanced Micro Devices, Inc.
+ *
+ * Vendor CSR block that gates the ARM SMMUv3 wired interrupts. Chains
+ * off the single GIC parent line and demultiplexes the CSR status into
+ * three children (eventq, gerror, priq), each enabled/acked
+ * via SMMU_CSR_IER/IDR/ISR that the ARM SMMUv3 driver requests individually.
+ */
+
+#include <linux/bitops.h>
+#include <linux/io.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+/* Interrupt types */
+#define SMMU_INTR_EVENT		BIT(0)
+#define SMMU_INTR_GLOBAL	BIT(2)
+#define SMMU_INTR_PRI		BIT(3)
+
+/* Mask for all SMMU interrupts */
+#define SMMU_INTR_ALL		(SMMU_INTR_EVENT | \
+				 SMMU_INTR_GLOBAL | SMMU_INTR_PRI)
+
+#define SMMU_CSR_ISR		0x24	/* Interrupt Status */
+#define SMMU_CSR_IER		0x2c	/* Interrupt Enable */
+#define SMMU_CSR_IDR		0x30	/* Interrupt Disable */
+
+/**
+ * struct xilinx_smmu_csr - SMMU CSR interrupt controller context
+ * @base:	MMIO base address of the CSR registers
+ * @domain:	Interrupt domain for the child interrupts
+ * @parent_irq:	parent (GIC) interrupt this block is chained to
+ * @lock:	protects the SMMU_CSR_IER/IDR/ISR read and writes
+ */
+struct xilinx_smmu_csr {
+	void __iomem		*base;
+	struct irq_domain	*domain;
+	int			parent_irq;
+	raw_spinlock_t		lock;
+};
+
+enum xilinx_smmu_csr_irq {
+	SMMU_CSR_IRQ_EVENTQ = 0,
+	SMMU_CSR_IRQ_GERROR = 2,
+	SMMU_CSR_IRQ_PRIQ = 3,
+	SMMU_CSR_IRQ_NR,
+};
+
+static u32 xilinx_smmu_csr_hwirq_mask(irq_hw_number_t hwirq)
+{
+	if (hwirq >= SMMU_CSR_IRQ_NR)
+		return 0;
+
+	return BIT(hwirq) & SMMU_INTR_ALL;
+}
+
+static void xilinx_smmu_csr_irq_mask(struct irq_data *d)
+{
+	struct xilinx_smmu_csr *csr = irq_data_get_irq_chip_data(d);
+	u32 mask = xilinx_smmu_csr_hwirq_mask(d->hwirq);
+
+	if (!mask)
+		return;
+
+	guard(raw_spinlock)(&csr->lock);
+	writel(mask, csr->base + SMMU_CSR_IDR);
+}
+
+static void xilinx_smmu_csr_irq_unmask(struct irq_data *d)
+{
+	struct xilinx_smmu_csr *csr = irq_data_get_irq_chip_data(d);
+	u32 mask = xilinx_smmu_csr_hwirq_mask(d->hwirq);
+
+	if (!mask)
+		return;
+
+	guard(raw_spinlock)(&csr->lock);
+	writel(mask, csr->base + SMMU_CSR_IER);
+}
+
+static void xilinx_smmu_csr_irq_ack(struct irq_data *d)
+{
+	struct xilinx_smmu_csr *csr = irq_data_get_irq_chip_data(d);
+	u32 mask = xilinx_smmu_csr_hwirq_mask(d->hwirq);
+
+	if (!mask)
+		return;
+
+	guard(raw_spinlock)(&csr->lock);
+	writel(mask, csr->base + SMMU_CSR_ISR);
+}
+
+static struct irq_chip xilinx_smmu_csr_chip = {
+	.name		= "xlnx-smmu-csr",
+	.irq_mask	= xilinx_smmu_csr_irq_mask,
+	.irq_unmask	= xilinx_smmu_csr_irq_unmask,
+	.irq_ack	= xilinx_smmu_csr_irq_ack,
+};
+
+static void xilinx_smmu_csr_irq_handler(struct irq_desc *desc)
+{
+	struct xilinx_smmu_csr *csr = irq_desc_get_handler_data(desc);
+	struct irq_chip *chip = irq_desc_get_chip(desc);
+	u32 status, pending;
+
+	chained_irq_enter(chip, desc);
+
+	status = readl(csr->base + SMMU_CSR_ISR);
+
+	/* Only service sources we support; SMMU_CSR_ISR latches raw status */
+	pending = status & SMMU_INTR_ALL;
+
+	while (pending) {
+		irq_hw_number_t hwirq = __ffs(pending);
+		int ret;
+
+		ret = generic_handle_domain_irq(csr->domain, hwirq);
+		if (ret)
+			pr_err_ratelimited("xilinx-smmu-csr: Failed to handle domain IRQ %lu: %d\n",
+					   hwirq, ret);
+
+		pending &= ~BIT(hwirq);
+	}
+
+	chained_irq_exit(chip, desc);
+}
+
+static int xilinx_smmu_csr_domain_map(struct irq_domain *d, unsigned int virq,
+				      irq_hw_number_t hwirq)
+{
+	struct xilinx_smmu_csr *csr = d->host_data;
+
+	if (!xilinx_smmu_csr_hwirq_mask(hwirq))
+		return -EINVAL;
+
+	irq_set_chip_and_handler(virq, &xilinx_smmu_csr_chip, handle_level_irq);
+	irq_set_chip_data(virq, csr);
+	irq_set_status_flags(virq, IRQ_LEVEL);
+
+	return 0;
+}
+
+static const struct irq_domain_ops xilinx_smmu_csr_domain_ops = {
+	.map	= xilinx_smmu_csr_domain_map,
+	.xlate	= irq_domain_xlate_onecell,
+};
+
+static int __init xilinx_smmu_csr_init(struct device_node *node, struct device_node *parent)
+{
+	struct xilinx_smmu_csr *csr;
+	int ret;
+
+	if (WARN_ON_ONCE(!parent))
+		return -EINVAL;
+
+	if (irq_find_matching_fwnode(of_fwnode_handle(node),
+				     DOMAIN_BUS_ANY))
+		return -ENODEV;
+
+	csr = kzalloc(sizeof(*csr), GFP_KERNEL);
+	if (!csr)
+		return -ENOMEM;
+
+	raw_spin_lock_init(&csr->lock);
+
+	csr->base = of_iomap(node, 0);
+	if (!csr->base) {
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	/* Start from a known state: all sources disabled, latches cleared. */
+	writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_IDR);
+	writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_ISR);
+
+	csr->domain = irq_domain_create_linear(of_fwnode_handle(node), SMMU_CSR_IRQ_NR,
+					       &xilinx_smmu_csr_domain_ops,
+					       csr);
+	if (!csr->domain) {
+		pr_err("%pOF: failed to create irq domain\n", node);
+		ret = -ENOMEM;
+		goto unmap;
+	}
+
+	csr->parent_irq = irq_of_parse_and_map(node, 0);
+	if (!csr->parent_irq) {
+		pr_err("%pOF: failed to map parent irq\n", node);
+		ret = -EINVAL;
+		goto remove_domain;
+	}
+
+	irq_set_chained_handler_and_data(csr->parent_irq,
+					 xilinx_smmu_csr_irq_handler, csr);
+
+	pr_debug("%pOF: Xilinx SMMU CSR interrupt controller registered\n", node);
+
+	return 0;
+
+remove_domain:
+	irq_domain_remove(csr->domain);
+unmap:
+	iounmap(csr->base);
+free:
+	kfree(csr);
+	return ret;
+}
+
+IRQCHIP_DECLARE(xilinx_smmu_csr, "xlnx,versal-net-smmu-csr",
+		xilinx_smmu_csr_init);

-- 
2.34.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver
  2026-09-18  6:39 ` [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver Tushar Nimkar
@ 2026-09-18  6:48   ` sashiko-bot
  2026-09-23  7:10     ` Tushar Nimkar
  0 siblings, 1 reply; 5+ messages in thread
From: sashiko-bot @ 2026-09-18  6:48 UTC (permalink / raw)
  To: Tushar Nimkar; +Cc: devicetree, robh, conor+dt

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] irqchip: unrecoverable interrupt storm due to unhandled CMDQ_SYNC
- [Medium] irqchip: spurious handling of disabled interrupts causes log spam
--

commit bfa978fa90b50ee4257046a8002784c0f7fc45f7
Author: Anirudha Sarangi <anirudha.sarangi@amd.com>

irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver

The Xilinx Versal NET SoC implements a Control and Status Register (CSR)
block in front of the ARM SMMUv3 to manage EVENTQ, PRIQ and GLOBAL
interrupts. This commit adds an irqchip driver to demultiplex these
interrupts and chain them to the upstream SMMUv3 driver.

> diff --git a/drivers/irqchip/irq-xilinx-smmu-csr.c b/drivers/irqchip/irq-xilinx-smmu-csr.c
> new file mode 100644
> index 0000000000000..d9b575c6c58b0
> --- /dev/null
> +++ b/drivers/irqchip/irq-xilinx-smmu-csr.c
[ ... ]
> +static void xilinx_smmu_csr_irq_handler(struct irq_desc *desc)
> +{
> +	struct xilinx_smmu_csr *csr = irq_desc_get_handler_data(desc);
> +	struct irq_chip *chip = irq_desc_get_chip(desc);
> +	u32 status, pending;
> +
> +	chained_irq_enter(chip, desc);
> +
> +	status = readl(csr->base + SMMU_CSR_ISR);
> +
> +	/* Only service sources we support; SMMU_CSR_ISR latches raw status */
> +	pending = status & SMMU_INTR_ALL;

[Severity: High]
If the CMDQ_SYNC interrupt (bit 1) was enabled or asserted by a previous boot
stage like u-boot or kexec, it is filtered out of this pending mask. Because
it is never cleared or acknowledged, will this cause an interrupt storm on the
parent GIC line and a system hard lockup?

[Severity: Medium]
Additionally, since SMMU_CSR_ISR latches the raw status, does masking against
SMMU_INTR_ALL instead of a shadow mask of actually enabled interrupts lead to
processing disabled or unmapped events? 

> +
> +	while (pending) {
> +		irq_hw_number_t hwirq = __ffs(pending);
> +		int ret;
> +
> +		ret = generic_handle_domain_irq(csr->domain, hwirq);
> +		if (ret)
> +			pr_err_ratelimited("xilinx-smmu-csr: Failed to handle domain IRQ %lu: %d\n",
> +					   hwirq, ret);

If an unmapped interrupt fires, generic_handle_domain_irq() will fail with
-EINVAL and log this error. Because the driver cannot ACK an unmapped
interrupt, the bit remains set in the ISR. Does this result in permanent
ratelimited log spam and wasted CPU cycles on every subsequent valid
interrupt?

> +
> +		pending &= ~BIT(hwirq);
> +	}
> +
> +	chained_irq_exit(chip, desc);
> +}
[ ... ]
> +static int __init xilinx_smmu_csr_init(struct device_node *node, struct device_node *parent)
> +{
[ ... ]
> +	/* Start from a known state: all sources disabled, latches cleared. */
> +	writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_IDR);
> +	writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_ISR);

[Severity: High]
SMMU_INTR_ALL omits bit 1 (CMDQ_SYNC). If a prior boot stage left CMDQ_SYNC
enabled or pending, doesn't writing only SMMU_INTR_ALL fail to disable and
clear it, setting up the interrupt storm described above?

> +
> +	csr->domain = irq_domain_create_linear(of_fwnode_handle(node), SMMU_CSR_IRQ_NR,
> +					       &xilinx_smmu_csr_domain_ops,
> +					       csr);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918-smmu-changes-v2-v2-0-adda681ab983@amd.com?part=2

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver
  2026-09-18  6:48   ` sashiko-bot
@ 2026-09-23  7:10     ` Tushar Nimkar
  0 siblings, 0 replies; 5+ messages in thread
From: Tushar Nimkar @ 2026-09-23  7:10 UTC (permalink / raw)
  To: sashiko-reviews, Tushar Nimkar; +Cc: devicetree, robh, conor+dt


On 9/18/2026 12:18 PM, sashiko-bot@kernel.org wrote:
> [You don't often get email from sashiko-bot@kernel.org. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] irqchip: unrecoverable interrupt storm due to unhandled CMDQ_SYNC
> - [Medium] irqchip: spurious handling of disabled interrupts causes log spam
Thanks Sashiko for the review,  all issues addressed in v3 and validated.
> --
>
> commit bfa978fa90b50ee4257046a8002784c0f7fc45f7
> Author: Anirudha Sarangi <anirudha.sarangi@amd.com>
>
> irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver
>
> The Xilinx Versal NET SoC implements a Control and Status Register (CSR)
> block in front of the ARM SMMUv3 to manage EVENTQ, PRIQ and GLOBAL
> interrupts. This commit adds an irqchip driver to demultiplex these
> interrupts and chain them to the upstream SMMUv3 driver.
>
>> diff --git a/drivers/irqchip/irq-xilinx-smmu-csr.c b/drivers/irqchip/irq-xilinx-smmu-csr.c
>> new file mode 100644
>> index 0000000000000..d9b575c6c58b0
>> --- /dev/null
>> +++ b/drivers/irqchip/irq-xilinx-smmu-csr.c
> [ ... ]
>> +static void xilinx_smmu_csr_irq_handler(struct irq_desc *desc)
>> +{
>> +     struct xilinx_smmu_csr *csr = irq_desc_get_handler_data(desc);
>> +     struct irq_chip *chip = irq_desc_get_chip(desc);
>> +     u32 status, pending;
>> +
>> +     chained_irq_enter(chip, desc);
>> +
>> +     status = readl(csr->base + SMMU_CSR_ISR);
>> +
>> +     /* Only service sources we support; SMMU_CSR_ISR latches raw status */
>> +     pending = status & SMMU_INTR_ALL;
> [Severity: High]
> If the CMDQ_SYNC interrupt (bit 1) was enabled or asserted by a previous boot
> stage like u-boot or kexec, it is filtered out of this pending mask. Because
> it is never cleared or acknowledged, will this cause an interrupt storm on the
> parent GIC line and a system hard lockup?
Yes — fixed in v3. The handler now gates on the hardware mask register 
and defensively masks+clears any active source it doesn't demultiplex 
(e.g. CMDQ_SYNC), so it can't hold the GIC
line asserted.
> [Severity: Medium]
> Additionally, since SMMU_CSR_ISR latches the raw status, does masking against
> SMMU_INTR_ALL instead of a shadow mask of actually enabled interrupts lead to
> processing disabled or unmapped events?
Fixed in v3 - dispatch is now gated on active = status & ~mask using the 
hardware mask register (SMMU_CSR_MASK), so masked/disabled-but-latched 
sources are never processed.
>> +
>> +     while (pending) {
>> +             irq_hw_number_t hwirq = __ffs(pending);
>> +             int ret;
>> +
>> +             ret = generic_handle_domain_irq(csr->domain, hwirq);
>> +             if (ret)
>> +                     pr_err_ratelimited("xilinx-smmu-csr: Failed to handle domain IRQ %lu: %d\n",
>> +                                        hwirq, ret);
> If an unmapped interrupt fires, generic_handle_domain_irq() will fail with
> -EINVAL and log this error. Because the driver cannot ACK an unmapped
> interrupt, the bit remains set in the ISR. Does this result in permanent
> ratelimited log spam and wasted CPU cycles on every subsequent valid
> interrupt?
Fixed in v3 -  On an error return the handler now masks+clears that 
source instead of logging, so the bit can't stay set
>> +
>> +             pending &= ~BIT(hwirq);
>> +     }
>> +
>> +     chained_irq_exit(chip, desc);
>> +}
> [ ... ]
>> +static int __init xilinx_smmu_csr_init(struct device_node *node, struct device_node *parent)
>> +{
> [ ... ]
>> +     /* Start from a known state: all sources disabled, latches cleared. */
>> +     writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_IDR);
>> +     writel(SMMU_INTR_ALL, csr->base + SMMU_CSR_ISR);
> [Severity: High]
> SMMU_INTR_ALL omits bit 1 (CMDQ_SYNC). If a prior boot stage left CMDQ_SYNC
> enabled or pending, doesn't writing only SMMU_INTR_ALL fail to disable and
> clear it, setting up the interrupt storm described above?
Correct — fixed in v3. Init now masks and clears every source the block 
implements (SMMU_INTR_HW_ALL = GENMASK(3,0)), including CMDQ_SYNC:

   writel(SMMU_INTR_HW_ALL, csr->base + SMMU_CSR_IDR); /* mask all  */
   writel(SMMU_INTR_HW_ALL, csr->base + SMMU_CSR_ISR); /* clear all */

so an inherited enabled/pending CMDQ_SYNC is disabled and cleared before 
the parent line is enabled.

>> +
>> +     csr->domain = irq_domain_create_linear(of_fwnode_handle(node), SMMU_CSR_IRQ_NR,
>> +                                            &xilinx_smmu_csr_domain_ops,
>> +                                            csr);
> [ ... ]
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260918-smmu-changes-v2-v2-0-adda681ab983@amd.com?part=2
Thanks, Tushar Nimkar

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-09-23  7:10 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-18  6:39 [PATCH v2 0/2] Add Xilinx Versal NET SMMU CSR interrupt controller support Tushar Nimkar
2026-09-18  6:39 ` [PATCH v2 1/2] dt-bindings: interrupt-controller: Add Xilinx Versal NET SMMU CSR Tushar Nimkar
2026-09-18  6:39 ` [PATCH v2 2/2] irqchip: Add Xilinx Versal NET SMMU CSR interrupt controller driver Tushar Nimkar
2026-09-18  6:48   ` sashiko-bot
2026-09-23  7:10     ` Tushar Nimkar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox