Hi Volodymyr,
On Sat, Aug 23, 2025 at 8:48 PM Volodymyr Babchuk
<volodymyr_babc...@epam.com> wrote:
>
>
> Hi Mykola,
>
> Mykola Kvach <xakep.ama...@gmail.com> writes:
>
> > From: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
> >
> > Store and restore active context and micro-TLB registers.
> >
> > Tested on R-Car H3 Starter Kit.
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshche...@epam.com>
> > Signed-off-by: Mykola Kvach <mykola_kv...@epam.com>
> > ---
> > xen/drivers/passthrough/arm/ipmmu-vmsa.c | 269 +++++++++++++++++++++++
> > 1 file changed, 269 insertions(+)
> >
> > diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > index dac0dd6d46..ced762657a 100644
> > --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > @@ -58,6 +58,8 @@
> > #define dev_print(dev, lvl, fmt, ...) \
> > printk(lvl "ipmmu: %s: " fmt, dev_name(dev), ## __VA_ARGS__)
> >
> > +#define dev_dbg(dev, fmt, ...) \
> > + dev_print(dev, XENLOG_DEBUG, fmt, ## __VA_ARGS__)
> > #define dev_info(dev, fmt, ...) \
> > dev_print(dev, XENLOG_INFO, fmt, ## __VA_ARGS__)
> > #define dev_warn(dev, fmt, ...) \
> > @@ -117,6 +119,23 @@ struct ipmmu_features {
> > unsigned int imuctr_ttsel_mask;
> > };
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +struct hw_register {
> > + const char *reg_name;
>
> Do you really need to store register name? It is not used anywhere.
No, it’s not needed. We can remove it.
Thank you for catching that.
>
> > + unsigned int reg_offset;
> > + unsigned int reg_data;
> > +};
> > +
> > +struct ipmmu_vmsa_backup {
> > + struct device *dev;
> > + unsigned int *utlbs_val;
> > + unsigned int *asids_val;
> > + struct list_head list;
> > +};
> > +
> > +#endif
> > +
> > /* Root/Cache IPMMU device's information */
> > struct ipmmu_vmsa_device {
> > struct device *dev;
> > @@ -129,6 +148,9 @@ struct ipmmu_vmsa_device {
> > struct ipmmu_vmsa_domain *domains[IPMMU_CTX_MAX];
> > unsigned int utlb_refcount[IPMMU_UTLB_MAX];
> > const struct ipmmu_features *features;
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + struct hw_register *reg_backup[IPMMU_CTX_MAX];
> > +#endif
> > };
> >
> > /*
> > @@ -534,6 +556,235 @@ static void ipmmu_domain_free_context(struct
> > ipmmu_vmsa_device *mmu,
> > spin_unlock_irqrestore(&mmu->lock, flags);
> > }
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +static DEFINE_SPINLOCK(ipmmu_devices_backup_lock);
> > +static LIST_HEAD(ipmmu_devices_backup);
> > +
> > +#define HW_REGISTER_BACKUP_SIZE 4
> > +
> > +static struct hw_register
> > root_pgtable[IPMMU_CTX_MAX][HW_REGISTER_BACKUP_SIZE] = {
> > + [0 ... (IPMMU_CTX_MAX - 1)] = {
> > + {"IMTTLBR0", IMTTLBR0, 0},
> > + {"IMTTUBR0", IMTTUBR0, 0},
> > + {"IMTTBCR", IMTTBCR, 0},
> > + {"IMCTR", IMCTR, 0},
>
> Taking into account that only 4 registers needs to be saved, will it be
> easier and more efficient to have a hardcoded struct like this?
>
> struct ipmmu_reg_ctx {
> unsigned int imttlbr0;
> unsigned int imttubr0;
> unsigned int imttbcr;
> unsigned int imctr;
> }
>
> instead of hw_register[] ?
>
> Especially taking into account that struct ipmmu_vmsa_backup{} does
> exactly this.
I see no problem with doing that.
>
>
> > + }
> > +};
> > +
> > +static uint32_t ipmmu_imuasid_read(struct ipmmu_vmsa_device *mmu,
> > + unsigned int utlb)
> > +{
> > + return ipmmu_read(mmu, ipmmu_utlb_reg(mmu, IMUASID(utlb)));
> > +}
> > +
> > +static void ipmmu_utlbs_backup(struct ipmmu_vmsa_device *mmu)
> > +{
> > + struct ipmmu_vmsa_backup *backup_data;
> > +
> > + dev_dbg(mmu->dev, "Handle micro-TLBs backup\n");
> > +
> > + spin_lock(&ipmmu_devices_backup_lock);
> > +
> > + list_for_each_entry( backup_data, &ipmmu_devices_backup, list )
> > + {
> > + struct iommu_fwspec *fwspec =
> > dev_iommu_fwspec_get(backup_data->dev);
> > + unsigned int i;
> > +
> > + if ( to_ipmmu(backup_data->dev) != mmu )
> > + continue;
> > +
> > + for ( i = 0; i < fwspec->num_ids; i++ )
> > + {
> > + unsigned int utlb = fwspec->ids[i];
> > +
> > + backup_data->asids_val[i] = ipmmu_imuasid_read(mmu, utlb);
> > + backup_data->utlbs_val[i] = ipmmu_imuctr_read(mmu, utlb);
> > + }
> > + }
> > +
> > + spin_unlock(&ipmmu_devices_backup_lock);
> > +}
> > +
> > +static void ipmmu_utlbs_restore(struct ipmmu_vmsa_device *mmu)
> > +{
> > + struct ipmmu_vmsa_backup *backup_data;
> > +
> > + dev_dbg(mmu->dev, "Handle micro-TLBs restore\n");
> > +
> > + spin_lock(&ipmmu_devices_backup_lock);
> > +
> > + list_for_each_entry( backup_data, &ipmmu_devices_backup, list )
> > + {
> > + struct iommu_fwspec *fwspec =
> > dev_iommu_fwspec_get(backup_data->dev);
> > + unsigned int i;
> > +
> > + if ( to_ipmmu(backup_data->dev) != mmu )
> > + continue;
> > +
> > + for ( i = 0; i < fwspec->num_ids; i++ )
> > + {
> > + unsigned int utlb = fwspec->ids[i];
> > +
> > + ipmmu_imuasid_write(mmu, utlb, backup_data->asids_val[i]);
> > + ipmmu_imuctr_write(mmu, utlb, backup_data->utlbs_val[i]);
> > + }
> > + }
> > +
> > + spin_unlock(&ipmmu_devices_backup_lock);
> > +}
> > +
> > +static void ipmmu_domain_backup_context(struct ipmmu_vmsa_domain *domain)
> > +{
> > + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> > + struct hw_register *reg = mmu->reg_backup[domain->context_id];
> > + unsigned int i;
> > +
> > + dev_dbg(mmu->dev, "Handle domain context %u backup\n",
> > domain->context_id);
> > +
> > + for ( i = 0; i < HW_REGISTER_BACKUP_SIZE; i++ )
> > + reg[i].reg_data = ipmmu_ctx_read_root(domain, reg[i].reg_offset);
> > +}
> > +
> > +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
> > +{
> > + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> > + struct hw_register *reg = mmu->reg_backup[domain->context_id];
> > + unsigned int i;
> > +
> > + dev_dbg(mmu->dev, "Handle domain context %u restore\n",
> > domain->context_id);
> > +
> > + for ( i = 0; i < HW_REGISTER_BACKUP_SIZE; i++ )
> > + {
> > + if ( reg[i].reg_offset != IMCTR )
> > + ipmmu_ctx_write_root(domain, reg[i].reg_offset,
> > reg[i].reg_data);
> > + else
> > + ipmmu_ctx_write_all(domain, reg[i].reg_offset,
> > + reg[i].reg_data | IMCTR_FLUSH);
> > + }
> > +}
> > +
> > +/*
> > + * Xen: Unlike Linux implementation, Xen uses a single driver instance
> > + * for handling all IPMMUs. There is no framework for ipmmu_suspend/resume
> > + * callbacks to be invoked for each IPMMU device. So, we need to iterate
> > + * through all registered IPMMUs performing required actions.
> > + *
> > + * Also take care of restoring special settings, such as translation
> > + * table format, etc.
> > + */
> > +static int __must_check ipmmu_suspend(void)
> > +{
> > + struct ipmmu_vmsa_device *mmu;
> > +
> > + if ( !iommu_enabled )
> > + return 0;
> > +
> > + printk(XENLOG_DEBUG "ipmmu: Suspending ...\n");
> > +
> > + spin_lock(&ipmmu_devices_lock);
> > +
> > + list_for_each_entry( mmu, &ipmmu_devices, list )
> > + {
> > + if ( ipmmu_is_root(mmu) )
> > + {
> > + unsigned int i;
> > +
> > + for ( i = 0; i < mmu->num_ctx; i++ )
> > + {
> > + if ( !mmu->domains[i] )
> > + continue;
> > + ipmmu_domain_backup_context(mmu->domains[i]);
> > + }
> > + }
> > + else
> > + ipmmu_utlbs_backup(mmu);
> > + }
> > +
> > + spin_unlock(&ipmmu_devices_lock);
> > +
> > + return 0;
> > +}
> > +
> > +static void ipmmu_resume(void)
> > +{
> > + struct ipmmu_vmsa_device *mmu;
> > +
> > + if ( !iommu_enabled )
> > + return;
> > +
> > + printk(XENLOG_DEBUG "ipmmu: Resuming ...\n");
> > +
> > + spin_lock(&ipmmu_devices_lock);
> > +
> > + list_for_each_entry( mmu, &ipmmu_devices, list )
> > + {
> > + uint32_t reg;
> > +
> > + /* Do not use security group function */
> > + reg = IMSCTLR + mmu->features->control_offset_base;
> > + ipmmu_write(mmu, reg, ipmmu_read(mmu, reg) & ~IMSCTLR_USE_SECGRP);
> > +
> > + if ( ipmmu_is_root(mmu) )
> > + {
> > + unsigned int i;
> > +
> > + /* Use stage 2 translation table format */
> > + reg = IMSAUXCTLR + mmu->features->control_offset_base;
> > + ipmmu_write(mmu, reg, ipmmu_read(mmu, reg) | IMSAUXCTLR_S2PTE);
> > +
> > + for ( i = 0; i < mmu->num_ctx; i++ )
> > + {
> > + if ( !mmu->domains[i] )
> > + continue;
> > + ipmmu_domain_restore_context(mmu->domains[i]);
> > + }
> > + }
> > + else
> > + ipmmu_utlbs_restore(mmu);
> > + }
> > +
> > + spin_unlock(&ipmmu_devices_lock);
> > +}
> > +
> > +static int ipmmu_alloc_ctx_suspend(struct device *dev)
> > +{
> > + struct ipmmu_vmsa_backup *backup_data;
> > + unsigned int *utlbs_val, *asids_val;
> > + struct iommu_fwspec *fwspec = dev_iommu_fwspec_get(dev);
> > +
> > + utlbs_val = xzalloc_array(unsigned int, fwspec->num_ids);
> > + if ( !utlbs_val )
> > + return -ENOMEM;
> > +
> > + asids_val = xzalloc_array(unsigned int, fwspec->num_ids);
> > + if ( !asids_val )
> > + {
> > + xfree(utlbs_val);
> > + return -ENOMEM;
> > + }
> > +
> > + backup_data = xzalloc(struct ipmmu_vmsa_backup);
> > + if ( !backup_data )
> > + {
> > + xfree(utlbs_val);
> > + xfree(asids_val);
> > + return -ENOMEM;
> > + }
> > +
> > + backup_data->dev = dev;
> > + backup_data->utlbs_val = utlbs_val;
> > + backup_data->asids_val = asids_val;
> > +
> > + spin_lock(&ipmmu_devices_backup_lock);
> > + list_add(&backup_data->list, &ipmmu_devices_backup);
> > + spin_unlock(&ipmmu_devices_backup_lock);
> > +
> > + return 0;
> > +}
> > +
> > +#endif /* CONFIG_SYSTEM_SUSPEND */
> > +
> > static int ipmmu_domain_init_context(struct ipmmu_vmsa_domain *domain)
> > {
> > uint64_t ttbr;
> > @@ -546,6 +797,9 @@ static int ipmmu_domain_init_context(struct
> > ipmmu_vmsa_domain *domain)
> > return ret;
> >
> > domain->context_id = ret;
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + domain->mmu->root->reg_backup[ret] = root_pgtable[ret];
> > +#endif
> >
> > /*
> > * TTBR0
> > @@ -602,6 +856,9 @@ static void ipmmu_domain_destroy_context(struct
> > ipmmu_vmsa_domain *domain)
> > ipmmu_ctx_write_root(domain, IMCTR, IMCTR_FLUSH);
> > ipmmu_tlb_sync(domain);
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + domain->mmu->root->reg_backup[domain->context_id] = NULL;
> > +#endif
> > ipmmu_domain_free_context(domain->mmu->root, domain->context_id);
> > }
> >
> > @@ -1307,6 +1564,14 @@ static int ipmmu_add_device(u8 devfn, struct device
> > *dev)
> > /* Let Xen know that the master device is protected by an IOMMU. */
> > dt_device_set_protected(dev_to_dt(dev));
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + if ( ipmmu_alloc_ctx_suspend(dev) )
> > + {
> > + dev_err(dev, "Failed to allocate context for suspend\n");
> > + return -ENOMEM;
> > + }
> > +#endif
> > +
> > dev_info(dev, "Added master device (IPMMU %s micro-TLBs %u)\n",
> > dev_name(fwspec->iommu_dev), fwspec->num_ids);
> >
> > @@ -1372,6 +1637,10 @@ static const struct iommu_ops ipmmu_iommu_ops =
> > .unmap_page = arm_iommu_unmap_page,
> > .dt_xlate = ipmmu_dt_xlate,
> > .add_device = ipmmu_add_device,
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + .suspend = ipmmu_suspend,
> > + .resume = ipmmu_resume,
> > +#endif
> > };
> >
> > static __init int ipmmu_init(struct dt_device_node *node, const void *data)
>
> --
> WBR, Volodymyr
Best regards,
Mykola
