From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Store and restore active context and micro-TLB registers.
Tested on R-Car H3 Starter Kit.
Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
---
Changes in V6:
- refactor code related to hw_register struct, from now it's called
ipmmu_reg_ctx
---
xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
1 file changed, 257 insertions(+)
diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
index ea9fa9ddf3..0973559861 100644
--- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
+++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
@@ -71,6 +71,8 @@
})
#endif
+#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, ...) \
@@ -130,6 +132,24 @@ struct ipmmu_features {
unsigned int imuctr_ttsel_mask;
};
+#ifdef CONFIG_SYSTEM_SUSPEND
+
+struct ipmmu_reg_ctx {
+ unsigned int imttlbr0;
+ unsigned int imttubr0;
+ unsigned int imttbcr;
+ unsigned int imctr;
+};
+
+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;
@@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
+#endif
};
/*
@@ -547,6 +570,222 @@ 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);
+
+static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
+
+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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
+
+ dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
+
+ regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
+ regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
+ regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
+ regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
+}
+
+static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
+{
+ struct ipmmu_vmsa_device *mmu = domain->mmu->root;
+ struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
+
+ dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
+
+ ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
+ ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
+ ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
+ ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
@@ -559,6 +798,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
@@ -615,6 +857,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);
}
@@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
}
#endif
+#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);
@@ -1492,6 +1745,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)
--
2.48.1
On 02.09.25 01:10, Mykola Kvach wrote:
Hello Mykola
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> Store and restore active context and micro-TLB registers.
>
> Tested on R-Car H3 Starter Kit.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> ---
> Changes in V6:
> - refactor code related to hw_register struct, from now it's called
> ipmmu_reg_ctx
The updated version looks good, thanks. However, I have one
concern/request ...
> ---
> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
> 1 file changed, 257 insertions(+)
>
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index ea9fa9ddf3..0973559861 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -71,6 +71,8 @@
> })
> #endif
>
> +#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, ...) \
> @@ -130,6 +132,24 @@ struct ipmmu_features {
> unsigned int imuctr_ttsel_mask;
> };
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +struct ipmmu_reg_ctx {
> + unsigned int imttlbr0;
> + unsigned int imttubr0;
> + unsigned int imttbcr;
> + unsigned int imctr;
> +};
> +
> +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;
> @@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
> +#endif
> };
>
> /*
> @@ -547,6 +570,222 @@ 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);
> +
> +static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
> +
> +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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> +
> + dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
> +
> + regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
> + regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
> + regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
> + regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
> +}
> +
> +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
> +{
> + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> + struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> +
> + dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
> +
> + ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
> + ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
> + ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
> + ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
> @@ -559,6 +798,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
> @@ -615,6 +857,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);
> }
>
> @@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
> }
> #endif
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> + if ( ipmmu_alloc_ctx_suspend(dev) )
> + {
> + dev_err(dev, "Failed to allocate context for suspend\n");
> + return -ENOMEM;
> + }
> +#endif
... The initial version was based on the driver code without PCI
support, but it is now present. There is PCI-specific code above in this
function (not visible in the context) that performs some initialization,
allocation and device assignment. What I mean is that in case of the
suspend context allocation error here, we will need to undo these
actions (i.e. deassign device). I would move this context allocation
(whose probability to fail is much lower than what is done for PCI dev)
above the PCI-specific stuff, and perform the context freeing on the
error path.
> +
> dev_info(dev, "Added master device (IPMMU %s micro-TLBs %u)\n",
> dev_name(fwspec->iommu_dev), fwspec->num_ids);
>
> @@ -1492,6 +1745,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)
Hi Oleksandr,
On Wed, Sep 3, 2025 at 1:01 PM Oleksandr Tyshchenko
<Oleksandr_Tyshchenko@epam.com> wrote:
>
>
>
> On 02.09.25 01:10, Mykola Kvach wrote:
>
> Hello Mykola
>
>
> > From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >
> > Store and restore active context and micro-TLB registers.
> >
> > Tested on R-Car H3 Starter Kit.
> >
> > Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> > Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> > ---
> > Changes in V6:
> > - refactor code related to hw_register struct, from now it's called
> > ipmmu_reg_ctx
>
> The updated version looks good, thanks. However, I have one
> concern/request ...
>
> > ---
> > xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
> > 1 file changed, 257 insertions(+)
> >
> > diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > index ea9fa9ddf3..0973559861 100644
> > --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> > @@ -71,6 +71,8 @@
> > })
> > #endif
> >
> > +#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, ...) \
> > @@ -130,6 +132,24 @@ struct ipmmu_features {
> > unsigned int imuctr_ttsel_mask;
> > };
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > +
> > +struct ipmmu_reg_ctx {
> > + unsigned int imttlbr0;
> > + unsigned int imttubr0;
> > + unsigned int imttbcr;
> > + unsigned int imctr;
> > +};
> > +
> > +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;
> > @@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
> > +#endif
> > };
> >
> > /*
> > @@ -547,6 +570,222 @@ 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);
> > +
> > +static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
> > +
> > +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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> > +
> > + dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
> > +
> > + regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
> > + regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
> > + regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
> > + regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
> > +}
> > +
> > +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
> > +{
> > + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> > + struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> > +
> > + dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
> > +
> > + ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
> > + ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
> > + ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
> > + ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
> > @@ -559,6 +798,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
> > @@ -615,6 +857,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);
> > }
> >
> > @@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
> > }
> > #endif
> >
> > +#ifdef CONFIG_SYSTEM_SUSPEND
> > + if ( ipmmu_alloc_ctx_suspend(dev) )
> > + {
> > + dev_err(dev, "Failed to allocate context for suspend\n");
> > + return -ENOMEM;
> > + }
> > +#endif
>
> ... The initial version was based on the driver code without PCI
> support, but it is now present. There is PCI-specific code above in this
> function (not visible in the context) that performs some initialization,
> allocation and device assignment. What I mean is that in case of the
> suspend context allocation error here, we will need to undo these
> actions (i.e. deassign device). I would move this context allocation
> (whose probability to fail is much lower than what is done for PCI dev)
> above the PCI-specific stuff, and perform the context freeing on the
> error path.
Maybe it would be better just to add some checks to the suspend handler.
We could skip suspend in case the context is not available, and avoid
deallocating previously allocated stuff. This is similar to what is
done for GICs.
What do you think? Or do you prefer to destroy everything related to the
IOMMU here on error?
>
> > +
> > dev_info(dev, "Added master device (IPMMU %s micro-TLBs %u)\n",
> > dev_name(fwspec->iommu_dev), fwspec->num_ids);
> >
> > @@ -1492,6 +1745,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)
Best regards,
Mykola
On 03.09.25 13:25, Mykola Kvach wrote:
> Hi Oleksandr,
Hello Mykola
>
> On Wed, Sep 3, 2025 at 1:01 PM Oleksandr Tyshchenko
> <Oleksandr_Tyshchenko@epam.com> wrote:
>>
>>
>>
>> On 02.09.25 01:10, Mykola Kvach wrote:
>>
>> Hello Mykola
>>
>>
>>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>>
>>> Store and restore active context and micro-TLB registers.
>>>
>>> Tested on R-Car H3 Starter Kit.
>>>
>>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
>>> ---
>>> Changes in V6:
>>> - refactor code related to hw_register struct, from now it's called
>>> ipmmu_reg_ctx
>>
>> The updated version looks good, thanks. However, I have one
>> concern/request ...
>>
>>> ---
>>> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
>>> 1 file changed, 257 insertions(+)
>>>
>>> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>>> index ea9fa9ddf3..0973559861 100644
>>> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>>> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
>>> @@ -71,6 +71,8 @@
>>> })
>>> #endif
>>>
>>> +#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, ...) \
>>> @@ -130,6 +132,24 @@ struct ipmmu_features {
>>> unsigned int imuctr_ttsel_mask;
>>> };
>>>
>>> +#ifdef CONFIG_SYSTEM_SUSPEND
>>> +
>>> +struct ipmmu_reg_ctx {
>>> + unsigned int imttlbr0;
>>> + unsigned int imttubr0;
>>> + unsigned int imttbcr;
>>> + unsigned int imctr;
>>> +};
>>> +
>>> +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;
>>> @@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
>>> +#endif
>>> };
>>>
>>> /*
>>> @@ -547,6 +570,222 @@ 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);
>>> +
>>> +static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
>>> +
>>> +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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
>>> +
>>> + dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
>>> +
>>> + regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
>>> + regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
>>> + regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
>>> + regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
>>> +}
>>> +
>>> +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
>>> +{
>>> + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
>>> + struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
>>> +
>>> + dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
>>> +
>>> + ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
>>> + ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
>>> + ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
>>> + ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
>>> @@ -559,6 +798,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
>>> @@ -615,6 +857,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);
>>> }
>>>
>>> @@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
>>> }
>>> #endif
>>>
>>> +#ifdef CONFIG_SYSTEM_SUSPEND
>>> + if ( ipmmu_alloc_ctx_suspend(dev) )
>>> + {
>>> + dev_err(dev, "Failed to allocate context for suspend\n");
>>> + return -ENOMEM;
>>> + }
>>> +#endif
>>
>> ... The initial version was based on the driver code without PCI
>> support, but it is now present. There is PCI-specific code above in this
>> function (not visible in the context) that performs some initialization,
>> allocation and device assignment. What I mean is that in case of the
>> suspend context allocation error here, we will need to undo these
>> actions (i.e. deassign device). I would move this context allocation
>> (whose probability to fail is much lower than what is done for PCI dev)
>> above the PCI-specific stuff, and perform the context freeing on the
>> error path.
>
> Maybe it would be better just to add some checks to the suspend handler.
> We could skip suspend in case the context is not available, and avoid
> deallocating previously allocated stuff. This is similar to what is
> done for GICs.
>
> What do you think? Or do you prefer to destroy everything related to the
> IOMMU here on error?
I would prefer if we fail early here in ipmmu_add_device (and rollback
changes) rather than continue and fail later, other people might think
differently. I think, if we cannot simply allocate a memory for the
sctructures that situation is bad.
>
>>
>>> +
>>> dev_info(dev, "Added master device (IPMMU %s micro-TLBs %u)\n",
>>> dev_name(fwspec->iommu_dev), fwspec->num_ids);
>>>
>>> @@ -1492,6 +1745,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)
>
> Best regards,
> Mykola
>
On Wed, Sep 3, 2025 at 2:49 PM Oleksandr Tyshchenko <olekstysh@gmail.com> wrote:
>
>
>
> On 03.09.25 13:25, Mykola Kvach wrote:
> > Hi Oleksandr,
>
> Hello Mykola
>
> >
> > On Wed, Sep 3, 2025 at 1:01 PM Oleksandr Tyshchenko
> > <Oleksandr_Tyshchenko@epam.com> wrote:
> >>
> >>
> >>
> >> On 02.09.25 01:10, Mykola Kvach wrote:
> >>
> >> Hello Mykola
> >>
> >>
> >>> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >>>
> >>> Store and restore active context and micro-TLB registers.
> >>>
> >>> Tested on R-Car H3 Starter Kit.
> >>>
> >>> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> >>> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
> >>> ---
> >>> Changes in V6:
> >>> - refactor code related to hw_register struct, from now it's called
> >>> ipmmu_reg_ctx
> >>
> >> The updated version looks good, thanks. However, I have one
> >> concern/request ...
> >>
> >>> ---
> >>> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
> >>> 1 file changed, 257 insertions(+)
> >>>
> >>> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> >>> index ea9fa9ddf3..0973559861 100644
> >>> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> >>> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> >>> @@ -71,6 +71,8 @@
> >>> })
> >>> #endif
> >>>
> >>> +#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, ...) \
> >>> @@ -130,6 +132,24 @@ struct ipmmu_features {
> >>> unsigned int imuctr_ttsel_mask;
> >>> };
> >>>
> >>> +#ifdef CONFIG_SYSTEM_SUSPEND
> >>> +
> >>> +struct ipmmu_reg_ctx {
> >>> + unsigned int imttlbr0;
> >>> + unsigned int imttubr0;
> >>> + unsigned int imttbcr;
> >>> + unsigned int imctr;
> >>> +};
> >>> +
> >>> +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;
> >>> @@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
> >>> +#endif
> >>> };
> >>>
> >>> /*
> >>> @@ -547,6 +570,222 @@ 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);
> >>> +
> >>> +static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
> >>> +
> >>> +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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> >>> +
> >>> + dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
> >>> +
> >>> + regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
> >>> + regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
> >>> + regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
> >>> + regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
> >>> +}
> >>> +
> >>> +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
> >>> +{
> >>> + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> >>> + struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> >>> +
> >>> + dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
> >>> +
> >>> + ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
> >>> + ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
> >>> + ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
> >>> + ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
> >>> @@ -559,6 +798,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
> >>> @@ -615,6 +857,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);
> >>> }
> >>>
> >>> @@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
> >>> }
> >>> #endif
> >>>
> >>> +#ifdef CONFIG_SYSTEM_SUSPEND
> >>> + if ( ipmmu_alloc_ctx_suspend(dev) )
> >>> + {
> >>> + dev_err(dev, "Failed to allocate context for suspend\n");
> >>> + return -ENOMEM;
> >>> + }
> >>> +#endif
> >>
> >> ... The initial version was based on the driver code without PCI
> >> support, but it is now present. There is PCI-specific code above in this
> >> function (not visible in the context) that performs some initialization,
> >> allocation and device assignment. What I mean is that in case of the
> >> suspend context allocation error here, we will need to undo these
> >> actions (i.e. deassign device). I would move this context allocation
> >> (whose probability to fail is much lower than what is done for PCI dev)
> >> above the PCI-specific stuff, and perform the context freeing on the
> >> error path.
> >
> > Maybe it would be better just to add some checks to the suspend handler.
> > We could skip suspend in case the context is not available, and avoid
> > deallocating previously allocated stuff. This is similar to what is
> > done for GICs.
> >
> > What do you think? Or do you prefer to destroy everything related to the
> > IOMMU here on error?
>
> I would prefer if we fail early here in ipmmu_add_device (and rollback
> changes) rather than continue and fail later, other people might think
> differently. I think, if we cannot simply allocate a memory for the
> sctructures that situation is bad.
Got it, I’ll fix this in the next version of the patch series.
Thank you for pointing that out.
>
>
>
> >
> >>
> >>> +
> >>> dev_info(dev, "Added master device (IPMMU %s micro-TLBs %u)\n",
> >>> dev_name(fwspec->iommu_dev), fwspec->num_ids);
> >>>
> >>> @@ -1492,6 +1745,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)
> >
> > Best regards,
> > Mykola
> >
>
Best regards,
Mykola
Hi,
Mykola Kvach <xakep.amatop@gmail.com> writes:
> From: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
>
> Store and restore active context and micro-TLB registers.
>
> Tested on R-Car H3 Starter Kit.
>
> Signed-off-by: Oleksandr Tyshchenko <oleksandr_tyshchenko@epam.com>
> Signed-off-by: Mykola Kvach <mykola_kvach@epam.com>
Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>
> ---
> Changes in V6:
> - refactor code related to hw_register struct, from now it's called
> ipmmu_reg_ctx
> ---
> xen/drivers/passthrough/arm/ipmmu-vmsa.c | 257 +++++++++++++++++++++++
> 1 file changed, 257 insertions(+)
>
> diff --git a/xen/drivers/passthrough/arm/ipmmu-vmsa.c b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> index ea9fa9ddf3..0973559861 100644
> --- a/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> +++ b/xen/drivers/passthrough/arm/ipmmu-vmsa.c
> @@ -71,6 +71,8 @@
> })
> #endif
>
> +#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, ...) \
> @@ -130,6 +132,24 @@ struct ipmmu_features {
> unsigned int imuctr_ttsel_mask;
> };
>
> +#ifdef CONFIG_SYSTEM_SUSPEND
> +
> +struct ipmmu_reg_ctx {
> + unsigned int imttlbr0;
> + unsigned int imttubr0;
> + unsigned int imttbcr;
> + unsigned int imctr;
> +};
> +
> +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;
> @@ -142,6 +162,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 ipmmu_reg_ctx *reg_backup[IPMMU_CTX_MAX];
> +#endif
> };
>
> /*
> @@ -547,6 +570,222 @@ 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);
> +
> +static struct ipmmu_reg_ctx root_pgtable[IPMMU_CTX_MAX];
> +
> +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 ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> +
> + dev_dbg(mmu->dev, "Handle domain context %u backup\n", domain->context_id);
> +
> + regs->imttlbr0 = ipmmu_ctx_read_root(domain, IMTTLBR0);
> + regs->imttubr0 = ipmmu_ctx_read_root(domain, IMTTUBR0);
> + regs->imttbcr = ipmmu_ctx_read_root(domain, IMTTBCR);
> + regs->imctr = ipmmu_ctx_read_root(domain, IMCTR);
> +}
> +
> +static void ipmmu_domain_restore_context(struct ipmmu_vmsa_domain *domain)
> +{
> + struct ipmmu_vmsa_device *mmu = domain->mmu->root;
> + struct ipmmu_reg_ctx *regs = mmu->reg_backup[domain->context_id];
> +
> + dev_dbg(mmu->dev, "Handle domain context %u restore\n", domain->context_id);
> +
> + ipmmu_ctx_write_root(domain, IMTTLBR0, regs->imttlbr0);
> + ipmmu_ctx_write_root(domain, IMTTUBR0, regs->imttubr0);
> + ipmmu_ctx_write_root(domain, IMTTBCR, regs->imttbcr);
> + ipmmu_ctx_write_all(domain, IMCTR, regs->imctr | 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;
> @@ -559,6 +798,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
> @@ -615,6 +857,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);
> }
>
> @@ -1427,6 +1672,14 @@ static int ipmmu_add_device(u8 devfn, struct device *dev)
> }
> #endif
>
> +#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);
>
> @@ -1492,6 +1745,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
© 2016 - 2025 Red Hat, Inc.