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

[Luca Fancellu <Luca.Fancellu@xxxxxxx>]

Hi Mykola,

> On 21 May 2026, at 18:45, Mykola Kvach <xakep.amatop@xxxxxxxxx> wrote:
> 
> From: Mykola Kvach <mykola_kvach@xxxxxxxx>
> 
> Add system suspend/resume callbacks for the Arm SMMUv3 driver.
> 
> During suspend, configure GBPA to abort incoming transactions, disable the
> translation interface while keeping CMDQ enabled, issue CMD_SYNC to ensure
> all previously issued commands have completed, then disable the SMMU IRQs
> and SMMU.
> 
> Resume uses arm_smmu_device_reset() to reprogram the SMMU and re-enable
> translation and interrupt generation.
> 
> The IRQ setup split follows the approach from Pranjal Shrivastava's Linux
> arm-smmu-v3 runtime/system sleep series: IRQ handlers are requested once
> during probe, while reset/resume only restores SMMU hardware state and
> re-enables IRQ_CTRL.
> 
> Only the pieces relevant to Xen's currently supported SMMUv3 path are
> ported here. Xen documents SMMUv3 MSI and PCI ATS as unsupported and not
> compiled/tested, so this patch does not restore SMMU MSI IRQ_CFGn registers
> nor reinitialize ATS/PRI endpoints. If those paths become usable,
> suspend/resume will need corresponding MSI restore and ATS/PRI
> quiesce/reinit steps.
> 
> Link: https://lore.kernel.org/r/20260414194702.1229094-1-praan@xxxxxxxxxx/
> Based-on-patch-by: Pranjal Shrivastava <praan@xxxxxxxxxx>
> Signed-off-by: Mykola Kvach <mykola_kvach@xxxxxxxx>
> ---
> Changes in V10:
> - Disable SMMU interrupt generation during suspend before disabling the
>  SMMU interface, matching the resume/reset path which re-enables IRQ_CTRL.
> 
> Changes in V9:
> - Use CMD_SYNC in suspend instead of polling CMDQ_CONS, so the suspend
>  path waits for command completion rather than only command consumption.
> - Document that arm_smmu_setup_irqs() is probe-only and that future Xen
>  SMMUv3 MSI support will need to restore SMMU IRQ_CFGn registers on
>  resume.
> - Restore the reference to Pranjal's Linux runtime/system sleep series and
>  clarify that MSI/ATS/PRI resume handling is outside the supported Xen
>  path.
> - Prefix the subject with xen/arm for consistency with the rest of the
>  Arm suspend/resume series.
> 
> Changes in V8:
> - Honor ARM_SMMU_FEAT_SEV when draining the CMDQ during suspend, matching
>  the existing runtime CMD_SYNC path.
> - Fold the suspend rollback reset path into a helper and rename the error
>  reporting to describe suspend rollback rather than resume.
> - Treat SMMU reset failure during resume as fatal instead of logging and
>  continuing with a potentially unusable IOMMU.
> - cosmetic changes
> ---
> xen/drivers/passthrough/arm/smmu-v3.c | 186 +++++++++++++++++++++-----
> 1 file changed, 150 insertions(+), 36 deletions(-)
> 
> diff --git a/xen/drivers/passthrough/arm/smmu-v3.c 
> b/xen/drivers/passthrough/arm/smmu-v3.c
> index bf153227db..be8028c036 100644
> --- a/xen/drivers/passthrough/arm/smmu-v3.c
> +++ b/xen/drivers/passthrough/arm/smmu-v3.c
> @@ -1814,8 +1814,7 @@ static int arm_smmu_write_reg_sync(struct 
> arm_smmu_device *smmu, u32 val,
> }
> 
> /* GBPA is "special" */
> -static int __init arm_smmu_update_gbpa(struct arm_smmu_device *smmu,
> -                                       u32 set, u32 clr)
> +static int arm_smmu_update_gbpa(struct arm_smmu_device *smmu, u32 set, u32 
> clr)

Now this one and arm_smmu_device_reset loose __init also for 
!CONFIG_SYSTEM_SUSPEND,
but I’m not sure if in the codebase we are dealing with these kind of cases 
already or if it’s still ok
to let it be without __init anyway.

Reviewed-by: Luca Fancellu <luca.fancellu@xxxxxxx>

Cheers,
Luca