[PATCH v2 3/4] iommu/arm-smmu-v3: Add support for dirty tracking in domain alloc
Jason Gunthorpe
jgg at nvidia.com
Fri Mar 8 06:31:24 PST 2024
On Thu, Feb 22, 2024 at 09:49:22AM +0000, Shameer Kolothum wrote:
> From: Joao Martins <joao.m.martins at oracle.com>
>
> This provides all the infrastructure to enable dirty tracking if the
> hardware has the capability and domain alloc request for it.
>
> Please note, we still report no support for IOMMU_CAP_DIRTY_TRACKING
> as it will finally be enabled in a subsequent patch.
>
> Signed-off-by: Joao Martins <joao.m.martins at oracle.com>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi at huawei.com>
> ---
> drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 95 ++++++++++++++++-----
> include/linux/io-pgtable.h | 4 +
> 2 files changed, 77 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> index bd30739e3588..058bbb0dbe2e 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -43,6 +43,7 @@ MODULE_PARM_DESC(disable_msipolling,
> "Disable MSI-based polling for CMD_SYNC completion.");
>
> static struct iommu_ops arm_smmu_ops;
> +static struct iommu_dirty_ops arm_smmu_dirty_ops;
>
> enum arm_smmu_msi_index {
> EVTQ_MSI_INDEX,
> @@ -86,7 +87,8 @@ static struct arm_smmu_option_prop arm_smmu_options[] = {
>
> static void arm_smmu_rmr_install_bypass_ste(struct arm_smmu_device *smmu);
> static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
> - struct arm_smmu_device *smmu);
> + struct arm_smmu_device *smmu,
> + bool enable_dirty);
> static int arm_smmu_alloc_cd_tables(struct arm_smmu_master *master);
> static void arm_smmu_tlb_inv_all_s2(struct arm_smmu_domain *smmu_domain);
>
> @@ -2378,7 +2380,7 @@ static struct iommu_domain *arm_smmu_domain_alloc_paging(struct device *dev)
> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> int ret;
>
> - ret = arm_smmu_domain_finalise(smmu_domain, master->smmu);
> + ret = arm_smmu_domain_finalise(smmu_domain, master->smmu, false);
> if (ret) {
> kfree(smmu_domain);
> return ERR_PTR(ret);
> @@ -2445,10 +2447,11 @@ static void arm_smmu_domain_free(struct iommu_domain *domain)
> }
>
> static int arm_smmu_domain_finalise(struct arm_smmu_domain *smmu_domain,
> - struct arm_smmu_device *smmu)
> + struct arm_smmu_device *smmu,
> + bool enable_dirty)
It is possibly a bit more readable if this is a flags and the test is
on IOMMU_HWPT_ALLOC_DIRTY_TRACKING
> {
> int ret;
> - unsigned long ias, oas;
> + unsigned long ias;
Isn't ias unused too?
> @@ -3193,7 +3197,9 @@ arm_smmu_domain_alloc_user(struct device *dev, u32 flags,
> const struct iommu_user_data *user_data)
> {
> struct arm_smmu_master *master = dev_iommu_priv_get(dev);
> - const u32 paging_flags = IOMMU_HWPT_ALLOC_NEST_PARENT;
> + const u32 paging_flags = IOMMU_HWPT_ALLOC_NEST_PARENT |
> + IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> + bool enforce_dirty = flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
Ah you based this on the nesting series.. Once part 2 is done we
should try to figure out what the order should be..
> +static int arm_smmu_read_and_clear_dirty(struct iommu_domain *domain,
> + unsigned long iova, size_t size,
> + unsigned long flags,
> + struct iommu_dirty_bitmap *dirty)
> +{
> + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> + struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
> +
> + if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
> + return -EINVAL;
This is not possible, these ops are only installed on S1 domains.
> + if (WARN_ON_ONCE(!ops || !ops->read_and_clear_dirty))
> + return -ENODEV;
This is not really needed, if this is corrupted then this will have a
tidy crash:
> + return ops->read_and_clear_dirty(ops, iova, size, flags, dirty);
If you are worried then move the WARN_ON into the finalize function to
ensure tha tthe io_pgtable_ops is properly formed after creating it.
> +static int arm_smmu_set_dirty_tracking(struct iommu_domain *domain,
> + bool enabled)
> +{
> + struct arm_smmu_domain *smmu_domain = to_smmu_domain(domain);
> + struct io_pgtable_ops *ops = smmu_domain->pgtbl_ops;
> +
> + if (smmu_domain->stage != ARM_SMMU_DOMAIN_S1)
> + return -EINVAL;
> + if (WARN_ON_ONCE(!ops))
> + return -ENODEV;
Ditto for both of these
Otherwise it looks OK to me
Jason
More information about the linux-arm-kernel
mailing list