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

From: Jason Gunthorpe <jgg@ziepe.ca>
To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
Cc: "kvmarm@lists.linux.dev" <kvmarm@lists.linux.dev>,
	"iommu@lists.linux.dev" <iommu@lists.linux.dev>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	Linuxarm <linuxarm@huawei.com>,
	"kevin.tian@intel.com" <kevin.tian@intel.com>,
	"alex.williamson@redhat.com" <alex.williamson@redhat.com>,
	"maz@kernel.org" <maz@kernel.org>,
	"oliver.upton@linux.dev" <oliver.upton@linux.dev>,
	"will@kernel.org" <will@kernel.org>,
	"robin.murphy@arm.com" <robin.murphy@arm.com>,
	"jean-philippe@linaro.org" <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
Date: Thu, 8 Feb 2024 12:26:05 -0400	[thread overview]
Message-ID: <20240208162605.GU31743@ziepe.ca> (raw)
In-Reply-To: <8bfca4c5bfe7424691efdae0f686ece4@huawei.com>

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

Jason

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