On 2022-05-06 00:28, Tian, Kevin wrote:
From: Jason Gunthorpe
Sent: Thursday, May 5, 2022 11:33 PM
/*
- * If the group has been claimed already, do not re-attach the default
- * domain.
+ * New drivers should support default domains and so the
detach_dev() op
+ * will never be called. Otherwise the NULL domain indicates the
+ * translation for the group should be set so it will work with the
translation should be 'blocked'?
No, not blocked.
+ * platform DMA ops.
I didn't get the meaning of the last sentence.
It is as discussed with Robin, NULL means to return the group back to
some platform defined translation, and in some cases this means
returning back to work with the platform's DMA ops - presumably if it
isn't using the dma api.
This is clearer than the original text.
Perhaps we could just leave that sentence as "Otherwise the NULL domain
represents platform-specific behaviour."
+ /*
+ * Changing the domain is done by calling attach on the new domain.
+ * Drivers should implement this so that DMA is always translated by
what does 'this' mean?
The code below - attach_dev called in a loop for each dev in the group.
Yes.
+ * either the new, old, or a blocking domain. DMA should never
isn't the blocking domain passed in as the new domain?
Soemtimes. This is a statement about the required
atomicity. New/old/block are all valid translations during the
operation. Identity is not.
but new/old/block are not the same type of classifications. A group
has an old domain and a new domain at this transition point, and
both old/new domains have a domain type (dma, unmanged, block,
identity, etc.). Mixing them together only increases confusion here.
Good point - in particular I think the "DMA is always translated" part
would be more accurately said as "DMA is always managed". When we're
reattaching back to the default domain here, and it happens to be an
identity domain, then DMA *may* be untranslated, but in a manner that
we've knowingly chosen. The key point is that if the driver supports
core domains, then it should never have any in-between state that allows
access to anything *other* than the current domain or the new domain.
Thanks,
Robin.
So, I'm going to leave this patch as-is since it has been tested now
and we need to get the series back into iommu-next ASAP.
Agree. Just hope some improvements on the code comment.
But either way given no more code change:
Reviewed-by: Kevin Tian <kevin.t...@intel.com>
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
