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

[Mostafa Saleh]

Hi Jean,

On Mon, Sep 25, 2023 at 06:18:53PM +0100, Jean-Philippe Brucker wrote:
> 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.

Yes, the other free was comming from an experiment I was making to use
the IOMMU layer with guest VMs, so this is correct. Sorry about that.

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

Thanks!

> Thanks,
> Jean