RE: [RFC PATCH v2 6/7] iommu/arm-smmu-v3: Use KVM VMID for s2 stage

[Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>]

> -----Original Message-----
> From: Jason Gunthorpe <jgg@ziepe.ca>
> Sent: Thursday, February 8, 2024 4:26 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: kvmarm@lists.linux.dev; iommu@lists.linux.dev; linux-arm-
> kernel@lists.infradead.org; Linuxarm <linuxarm@huawei.com>;
> kevin.tian@intel.com; alex.williamson@redhat.com; maz@kernel.org;
> oliver.upton@linux.dev; will@kernel.org; robin.murphy@arm.com; jean-
> philippe@linaro.org; Jonathan Cameron <jonathan.cameron@huawei.com>
> Subject: Re: [RFC PATCH v2 6/7] iommu/arm-smmu-v3: Use KVM VMID for s2
> stage
> 
> On Thu, Feb 08, 2024 at 04:14:07PM +0000, Shameerali Kolothum Thodi wrote:
> > > >  #include <linux/of.h>
> > > > @@ -2399,9 +2400,13 @@ int arm_smmu_domain_alloc_id(struct
> > > arm_smmu_device *smmu,
> > > >  	} else if (smmu_domain->stage == ARM_SMMU_DOMAIN_S2) {
> > > >  		int vmid;
> > > >
> > > > -		/* Reserve VMID 0 for stage-2 bypass STEs */
> > > > -		vmid = ida_alloc_range(&smmu->vmid_map, 1,
> > > > -				       (1 << smmu->vmid_bits) - 1, GFP_KERNEL);
> > > > +		if (smmu_domain->kvm) {
> > > > +			vmid = kvm_pinned_vmid_get(smmu_domain->kvm);
> > > > +		} else {
> > > > +			/* Reserve VMID 0 for stage-2 bypass STEs */
> > > > +			vmid = ida_alloc_range(&smmu->vmid_map, 1,
> > > > +					       (1 << smmu->vmid_bits) - 1,
> > > GFP_KERNEL);
> > > > +		}
> > >
> > > We cannot allow the two different STEs to be programmed with the same
> > > VMID but different translations, so somehow the two allocators have to
> > > work together.
> >
> > My idea was, we only set smmu_domain->kvm in the nested parent case
> > and that means SMMUv3 supports both S1 & S2. So all the non-nested
> domains
> > are using S1.
> 
> Okay, that is what I was thinking to get to.
> 
> The logic needs to reflect this directly though:
> 
> /*
>  * There can only be one allocator for VMIDs active at once. If BTM is
>  * turned on then KVM's allocator always supplies the VMID, and the
>  * VMID is matched by CPU invalidation of the KVM S2. Right now there
>  * is no API to get an unused VMID from KVM so this also means BTM systems
>  * cannot support S2 without an associated KVM.
>  */
> if ((smmu->feature & ARM_SMMU_FEAT_BTM)) {
>    if (!smmu_domain->kvm)
>       vmid = -EOPNOTSUPP;
>    else
>        vmid = kvm_pinned_vmid_get(smmu_domain->kvm);
> } else {
> 	/* Reserve VMID 0 for stage-2 bypass STEs */
> 	vmid = ida_alloc_range(&smmu->vmid_map, 1,
> 		       (1 << smmu->vmid_bits) - 1, GFP_KERNEL);
> }
> 
> (and matching change to the free side)
> 
> > This is what I was trying to do. But not sure we have systems out there where
> > only S2  is supported and that may require a private VMID without KVM.
> 
> iommufd can ask for a nesting parent without supplying a KVM fd, which
> would cause vmid aliasing between the two allocators. That needs to be
> blocked as above

Ah..That's right. Missed that case where KVM can be NULL, but still there is a 
request for nested domain.

Thanks,
Shameer

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel