[PATCH v5 11/12] iommu/ipmmu-vmsa: Implement suspend/resume callbacks

Mykola Kvach posted 12 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v5 11/12] iommu/ipmmu-vmsa: Implement suspend/resume callbacks
Posted by Mykola Kvach 2 months, 2 weeks ago
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>
---
 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;
+    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},
+    }
+};
+
+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)
-- 
2.48.1
Re: [PATCH v5 11/12] iommu/ipmmu-vmsa: Implement suspend/resume callbacks
Posted by Volodymyr Babchuk 2 months, 1 week ago
Hi Mykola,

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>
> ---
>  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.

> +    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.


> +    }
> +};
> +
> +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
Re: [PATCH v5 11/12] iommu/ipmmu-vmsa: Implement suspend/resume callbacks
Posted by Mykola Kvach 2 months ago
Hi Volodymyr,

On Sat, Aug 23, 2025 at 8:48 PM Volodymyr Babchuk
<Volodymyr_Babchuk@epam.com> wrote:
>
>
> Hi Mykola,
>
> 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>
> > ---
> >  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