On Tue, Apr 09, 2024 at 10:11:28AM +0800, Baolu Lu wrote:
On 4/8/24 10:19 PM, Jason Gunthorpe wrote:I was talking specifically about the type field you suggested adding
On Sat, Apr 06, 2024 at 02:09:34PM +0800, Baolu Lu wrote:Sorry that I don't fully understand the proposal here.
On 4/3/24 7:59 PM, Jason Gunthorpe wrote:The only use for this is the PRI callbacks right? Maybe instead of
On Wed, Apr 03, 2024 at 09:15:12AM +0800, Lu Baolu wrote:Yes, you are right. For the SVA case, I will add the following changes.
+ /* A bond already exists, just take a reference`. */At least in this context this is not enough we need to ensure that the
+ handle = iommu_attach_handle_get(group, iommu_mm->pasid);
+ if (handle) {
+ mutex_unlock(&iommu_sva_lock);
+ return handle;
}
domain on the PASID is actually an SVA domain and it was installed by
this mechanism, not an iommufd domain for instance.
ie you probably need a type field in the iommu_attach_handle to tell
what the priv is.
Otherwise this seems like a great idea!
The IOMMUFD path will also need such enhancement. I will update it in
the next version.
adding a handle type let's just check domain->iopf_handler ?
Ie SVA will pass &ommu_sva_iopf_handler as its "type"
to the handle struct.
Instead of adding a type field check the domain->iopf_handler to
determine the domain and thus handle type.
The problem is that the context code (SVA, IOMMUFD, etc.) needs to makeRight, you have a derived struct for each user and you need a way to
sure that the attach handle is really what it has installed during
domain attachment. The context code needs some mechanism to include some
kind of "owner cookie" in the attach handle, so that it could check
against it later for valid use.
check if casting from the general handle struct to the derived struct
is OK.
I'm suggesting using domain->iopf_handle as the type key.