[PATCH v11 04/11] iommu/arm-smmu-v3: Add ARM_SMMU_FEAT_EVTQ for the event queue
Mostafa Saleh
smostafa at google.com
Tue Oct 20 04:16:54 PDT 2026
On Mon, Oct 05, 2026 at 12:30:47PM -0700, Nicolin Chen wrote:
> The driver programs and enables the event queue unconditionally, while the
> PRI queue has an ARM_SMMU_FEAT_PRI gating each of its touch points. Yet a
> kdump kernel wants to leave both of the queues alone, which would take an
> is_kdump_kernel() test at every one of those places.
>
> Add an ARM_SMMU_FEAT_EVTQ that the probe always sets, as the event queue is
> architecturally mandatory, and gate the queue's allocation, its interrupt,
> its MSI vector, and its CR0 and IRQ_CTRL enables on it. A subsequent change
> will turn the queue off in a single place.
>
> No functional change intended.
>
> Suggested-by: Jason Gunthorpe <jgg at nvidia.com>
> Suggested-by: Robin Murphy <robin.murphy at arm.com>
> Acked-by: Kiryl Shutsemau (Meta) <kas at kernel.org>
> Reviewed-by: Jason Gunthorpe <jgg at nvidia.com>
> Tested-by: Breno Leitao <leitao at kernel.org>
> Tested-by: Cristian Prundeanu <cpru at amazon.com>
> Assisted-by: LLM
> Signed-off-by: Nicolin Chen <nicolinc at nvidia.com>
Reviewed-by: Mostafa Saleh <smostafa at google.com>
There is a nit below though.
Thanks,
Mostafa
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 1 +
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 89 ++++++++++++++-------
> 2 files changed, 61 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> index 8f2c0b1cc4a8d..559bfa8cbc5e0 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
> @@ -972,6 +972,7 @@ struct arm_smmu_device {
> #define ARM_SMMU_FEAT_BBML2 (1 << 24)
> #define ARM_SMMU_FEAT_HAFT (1 << 25)
> #define ARM_SMMU_FEAT_DS (1 << 26)
> +#define ARM_SMMU_FEAT_EVTQ (1 << 27)
> u32 features;
>
> #define ARM_SMMU_OPT_SKIP_PREFETCH (1 << 0)
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index 6ff61565f7583..756825e004774 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -2381,7 +2381,8 @@ static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
> {
> struct arm_smmu_device *smmu = dev;
>
> - arm_smmu_evtq_thread(irq, dev);
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ)
> + arm_smmu_evtq_thread(irq, dev);
> if (smmu->features & ARM_SMMU_FEAT_PRI)
> arm_smmu_priq_thread(irq, dev);
>
> @@ -2390,7 +2391,12 @@ static irqreturn_t arm_smmu_combined_irq_thread(int irq, void *dev)
>
> static irqreturn_t arm_smmu_combined_irq_handler(int irq, void *dev)
> {
> - arm_smmu_gerror_handler(irq, dev);
> + struct arm_smmu_device *smmu = dev;
> + irqreturn_t ret = arm_smmu_gerror_handler(irq, dev);
> +
> + /* Without either queue, the thread would have nothing to drain */
> + if (!(smmu->features & (ARM_SMMU_FEAT_EVTQ | ARM_SMMU_FEAT_PRI)))
> + return ret;
> return IRQ_WAKE_THREAD;
> }
>
> @@ -4763,11 +4769,14 @@ static int arm_smmu_init_queues(struct arm_smmu_device *smmu)
> return ret;
>
> /* evtq */
> - ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, smmu->page1,
> - ARM_SMMU_EVTQ_PROD, ARM_SMMU_EVTQ_CONS,
> - EVTQ_ENT_DWORDS, "evtq");
> - if (ret)
> - return ret;
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ) {
> + ret = arm_smmu_init_one_queue(smmu, &smmu->evtq.q, smmu->page1,
> + ARM_SMMU_EVTQ_PROD,
> + ARM_SMMU_EVTQ_CONS,
> + EVTQ_ENT_DWORDS, "evtq");
> + if (ret)
> + return ret;
> + }
>
> if ((smmu->features & ARM_SMMU_FEAT_SVA) &&
> (smmu->features & ARM_SMMU_FEAT_STALLS)) {
> @@ -4950,7 +4959,15 @@ static void arm_smmu_setup_msis(struct arm_smmu_device *smmu)
>
> /* Clear the MSI address regs */
> writeq_relaxed(0, smmu->base + ARM_SMMU_GERROR_IRQ_CFG0);
> - writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
> +
> + /* PRIQ's vector needs EVTQ's to be kept */
> + WARN_ON_ONCE(!(smmu->features & ARM_SMMU_FEAT_EVTQ) &&
> + (smmu->features & ARM_SMMU_FEAT_PRI));
> +
I can't see how we can end up in this case, I’d suggest dropping it
if I am not missing something.
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ)
> + writeq_relaxed(0, smmu->base + ARM_SMMU_EVTQ_IRQ_CFG0);
> + else
> + nvec--;
>
> if (smmu->features & ARM_SMMU_FEAT_PRI)
> writeq_relaxed(0, smmu->base + ARM_SMMU_PRIQ_IRQ_CFG0);
> @@ -4987,16 +5004,20 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
> arm_smmu_setup_msis(smmu);
>
> /* Request interrupt lines */
> - irq = smmu->evtq.q.irq;
> - if (irq) {
> - ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
> - arm_smmu_evtq_thread,
> - IRQF_ONESHOT,
> - "arm-smmu-v3-evtq", smmu);
> - if (ret < 0)
> - dev_warn(smmu->dev, "failed to enable evtq irq\n");
> - } else {
> - dev_warn(smmu->dev, "no evtq irq - events will not be reported!\n");
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ) {
> + irq = smmu->evtq.q.irq;
> + if (irq) {
> + ret = devm_request_threaded_irq(smmu->dev, irq, NULL,
> + arm_smmu_evtq_thread,
> + IRQF_ONESHOT,
> + "arm-smmu-v3-evtq", smmu);
> + if (ret < 0)
> + dev_warn(smmu->dev,
> + "failed to enable evtq irq\n");
> + } else {
> + dev_warn(smmu->dev,
> + "no evtq irq - events will not be reported!\n");
> + }
> }
>
> irq = smmu->gerr_irq;
> @@ -5029,7 +5050,7 @@ static void arm_smmu_setup_unique_irqs(struct arm_smmu_device *smmu)
> static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
> {
> int ret, irq;
> - u32 irqen_flags = IRQ_CTRL_EVTQ_IRQEN | IRQ_CTRL_GERROR_IRQEN;
> + u32 irqen_flags = IRQ_CTRL_GERROR_IRQEN;
>
> /* Disable IRQs first */
> ret = arm_smmu_write_reg_sync(smmu, 0, ARM_SMMU_IRQ_CTRL,
> @@ -5055,6 +5076,8 @@ static int arm_smmu_setup_irqs(struct arm_smmu_device *smmu)
> } else
> arm_smmu_setup_unique_irqs(smmu);
>
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ)
> + irqen_flags |= IRQ_CTRL_EVTQ_IRQEN;
> if (smmu->features & ARM_SMMU_FEAT_PRI)
> irqen_flags |= IRQ_CTRL_PRIQ_IRQEN;
>
> @@ -5173,16 +5196,21 @@ static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> smmu, arm_smmu_make_cmd_op(CMDQ_OP_TLBI_NSNH_ALL));
>
> /* Event queue */
> - writeq_relaxed(smmu->evtq.q.q_base, smmu->base + ARM_SMMU_EVTQ_BASE);
> - writel_relaxed(smmu->evtq.q.llq.prod, smmu->page1 + ARM_SMMU_EVTQ_PROD);
> - writel_relaxed(smmu->evtq.q.llq.cons, smmu->page1 + ARM_SMMU_EVTQ_CONS);
> -
> - enables |= CR0_EVTQEN;
> - ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> - ARM_SMMU_CR0ACK);
> - if (ret) {
> - dev_err(smmu->dev, "failed to enable event queue\n");
> - return ret;
> + if (smmu->features & ARM_SMMU_FEAT_EVTQ) {
> + writeq_relaxed(smmu->evtq.q.q_base,
> + smmu->base + ARM_SMMU_EVTQ_BASE);
> + writel_relaxed(smmu->evtq.q.llq.prod,
> + smmu->page1 + ARM_SMMU_EVTQ_PROD);
> + writel_relaxed(smmu->evtq.q.llq.cons,
> + smmu->page1 + ARM_SMMU_EVTQ_CONS);
> +
> + enables |= CR0_EVTQEN;
> + ret = arm_smmu_write_reg_sync(smmu, enables, ARM_SMMU_CR0,
> + ARM_SMMU_CR0ACK);
> + if (ret) {
> + dev_err(smmu->dev, "failed to enable event queue\n");
> + return ret;
> + }
> }
>
> /* PRI queue */
> @@ -5330,6 +5358,9 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
> u32 reg;
> bool coherent = smmu->features & ARM_SMMU_FEAT_COHERENCY;
>
> + /* The event queue is architecturally mandatory, unlike the PRI queue */
> + smmu->features |= ARM_SMMU_FEAT_EVTQ;
> +
> /* IDR0 */
> reg = readl_relaxed(smmu->base + ARM_SMMU_IDR0);
>
> --
> 2.43.0
>
More information about the linux-arm-kernel
mailing list