Re: [PATCH v8 09/13] arm/smmu-v3: add suspend/resume handlers

[Mykola Kvach <xakep.amatop@xxxxxxxxx>]

On Fri, May 8, 2026 at 3:22 PM Luca Fancellu <Luca.Fancellu@xxxxxxx> wrote:
>
> HI Mykola,
>
> >>>
> >>> -static int __init arm_smmu_device_reset(struct arm_smmu_device *smmu)
> >>> +static int arm_smmu_device_reset(struct arm_smmu_device *smmu)
> >>> {
> >>> int ret;
> >>> u32 reg, enables;
> >>> @@ -2163,17 +2166,9 @@ static int __init arm_smmu_device_reset(struct 
> >>> arm_smmu_device *smmu)
> >>> }
> >>> }
> >>>
> >>> - ret = arm_smmu_setup_irqs(smmu);
> >>> - if (ret) {
> >>> - dev_err(smmu->dev, "failed to setup irqs\n");
> >>
> >> We are moving this one to the probe and ..
> >>
> >>> + ret = arm_smmu_enable_irqs(smmu);
> >>> + if ( ret )
> >>
> >> changing with this one, but arm_smmu_setup_irqs() also calls 
> >> arm_smmu_setup_unique_irqs() which
> >> calls arm_smmu_setup_msis(), are we sure that on resume we will get the 
> >> same state?
> >
> > This follows the split introduced in the Linux arm-smmu-v3 runtime/system 
> > sleep
> > series:
> >
> > https://lore.kernel.org/linux-iommu/20260414194702.1229094-1-praan@xxxxxxxxxx/
> >
> > The intent is to keep IRQ handler registration as one-time probe state, 
> > while
> > reset/resume only restores the SMMU hardware state and re-enables interrupt
> > generation.
> >
> > You are right that the MSI case needs extra care. In the Linux series this 
> > is
> > handled by arm_smmu_resume_msis(), which restores the SMMU-side MSI
> > configuration. I did not port that part in this patch because Xen SMMUv3 MSI
> > support is currently documented as unsupported and is not part of the
> > supported/tested path, so this patch only covers the wired IRQ path used by 
> > Xen
> > today.
> >
> > If Xen SMMUv3 MSI support becomes usable in the future, the resume path will
> > need an equivalent MSI restore step before IRQ_CTRL is re-enabled.
>
> In the mean time should we check maybe smmu->features doesn’t have
> ARM_SMMU_FEAT_MSI flag and document it in commit message?
>
> What do you think about it? I’m just worried someone uses CONFIG_MSI and your
> feature and ends up in some trouble, while we know that your feature breaks
> CONFIG_MSI.

Good point.

I don't think checking only ARM_SMMU_FEAT_MSI in this patch is the right
approach, since that reflects hardware capability rather than whether Xen is
actually using the SMMUv3 MSI IRQ path.

For v9, I plan to keep this SMMUv3 patch limited to documenting the current
limitation in the driver and in the commit message: the MSI IRQ path is not
host-suspend-safe yet because resume does not restore the SMMU *_IRQ_CFGn
registers.

The actual runtime block will be added in a later host suspend policy patch,
together with the other runtime blockers. That keeps the policy in one place
and ensures PSCI_FEATURES(SYSTEM_SUSPEND) stays consistent with the actual
SYSTEM_SUSPEND handling.

Best regards,
Mykola

>
> Maybe the maintainers can give their opinion here as well.
>
> Cheers,
> Luca