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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Mykola,

> On 8 May 2026, at 22:44, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> 
> 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.

Yes you are right, I realised that moments after sending my reply, a good check
would be to complain only if Xen is actually using that path.

Let’s go for documenting the limitation unless maintainers disagree.

Cheers,
Luca