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

Jean-Philippe Brucker
2023-02-08T18:15:11Z

On Tue, Feb 07, 2023 at 01:22:11PM +0000, Mostafa Saleh wrote:
> > +static struct iommu_domain *kvm_arm_smmu_domain_alloc(unsigned type)
> > +{
> > + struct kvm_arm_smmu_domain *kvm_smmu_domain;
> > +
> > + /*
> > + * We don't support
> > + * - IOMMU_DOMAIN_IDENTITY because we rely on the host telling the
> > + * hypervisor which pages are used for DMA.
> > + * - IOMMU_DOMAIN_DMA_FQ because lazy unmap would clash with memory
> > + * donation to guests.
> > + */
> > + if (type != IOMMU_DOMAIN_DMA &&
> > + type != IOMMU_DOMAIN_UNMANAGED)
> > + return NULL;
> > +
> > + kvm_smmu_domain = kzalloc(sizeof(*kvm_smmu_domain), GFP_KERNEL);
> > + if (!kvm_smmu_domain)
> > + return NULL;
> > +
> > + mutex_init(&kvm_smmu_domain->init_mutex);
> > +
> > + return &kvm_smmu_domain->domain;
> > +}
> > +
> > +static int kvm_arm_smmu_domain_finalize(struct kvm_arm_smmu_domain *kvm_smmu_domain,
> > + struct kvm_arm_smmu_master *master)
> > +{
> > + int ret = 0;
> > + struct page *p;
> > + unsigned long pgd;
> > + struct arm_smmu_device *smmu = master->smmu;
> > + struct host_arm_smmu_device *host_smmu = smmu_to_host(smmu);
> > +
> > + if (kvm_smmu_domain->smmu) {
> > + if (kvm_smmu_domain->smmu != smmu)
> > + return -EINVAL;
> > + return 0;
> > + }
> > +
> > + ret = ida_alloc_range(&kvm_arm_smmu_domain_ida, 0, 1 << smmu->vmid_bits,
> > + GFP_KERNEL);
> > + if (ret < 0)
> > + return ret;
> > + kvm_smmu_domain->id = ret;
> > +
> > + /*
> > + * PGD allocation does not use the memcache because it may be of higher
> > + * order when concatenated.
> > + */
> > + p = alloc_pages_node(dev_to_node(smmu->dev), GFP_KERNEL | __GFP_ZERO,
> > + host_smmu->pgd_order);
> > + if (!p)
> > + return -ENOMEM;
> > +
> > + pgd = (unsigned long)page_to_virt(p);
> > +
> > + local_lock_irq(&memcache_lock);
> > + ret = kvm_call_hyp_nvhe_mc(smmu, __pkvm_host_iommu_alloc_domain,
> > + host_smmu->id, kvm_smmu_domain->id, pgd);
>
> What is the idea of postponing this HVC to attach and don’t call it in
> alloc_domain HVC?

Yes ideally this HVC would be in kvm_arm_smmu_domain_alloc() above, but
due to the way the IOMMU API works at the moment, that function doesn't
take a context parameter. So we don't know which SMMU this will be for,
which is problematic. We don't know which page table formats are
supported, how many VMIDs, how to allocate memory for the page tables
(which NUMA node is the SMMU on, can it access high physical addresses or
does it need special allocations?)

I think there are plans to add a context to domain_alloc(), but at the
moment IOMMU drivers complete the domain allocation in attach().

Thanks,
Jean