Re: [RFC PATCH 40/45] iommu/arm-smmu-v3-kvm: Add IOMMU ops

Jean-Philippe Brucker
2023-09-25T17:19:24Z

On Wed, Sep 20, 2023 at 04:27:41PM +0000, Mostafa Saleh wrote:
> > +static void kvm_arm_smmu_domain_free(struct iommu_domain *domain)
> > +{
> > + int ret;
> > + struct kvm_arm_smmu_domain *kvm_smmu_domain = to_kvm_smmu_domain(domain);
> > + struct arm_smmu_device *smmu = kvm_smmu_domain->smmu;
> > +
> > + if (smmu) {
> > + struct host_arm_smmu_device *host_smmu = smmu_to_host(smmu);
> > +
> > + ret = kvm_call_hyp_nvhe(__pkvm_host_iommu_free_domain,
> > + host_smmu->id, kvm_smmu_domain->id);
> > + /*
> > + * On failure, leak the pgd because it probably hasn't been
> > + * reclaimed by the host.
> > + */
> > + if (!WARN_ON(ret))
> > + free_pages(kvm_smmu_domain->pgd, host_smmu->pgd_order);
> I believe this doube-free the pgd in case of attatch_dev fails, as it
> would try to free it their also (in kvm_arm_smmu_domain_finalize).
>
> I think this is right place to free the pgd.

Since this depends on kvm_smmu_domain->smmu being non-NULL, which is only
true if finalize() succeeded, then we shouldn't get a double-free.

But finalize() does leak kvm_smmu_domain->id if the pgd allocation fails,
I fixed that.

Thanks,
Jean