From: Nicolin Chen nicolinc@nvidia.com Sent: Friday, July 28, 2023 12:43 PM
On Fri, Jul 28, 2023 at 03:45:39AM +0000, Tian, Kevin wrote:
From: Nicolin Chen nicolinc@nvidia.com Sent: Friday, July 28, 2023 3:04 AM
On Thu, Jul 27, 2023 at 09:03:01AM -0300, Jason Gunthorpe wrote:
On Wed, Jul 26, 2023 at 07:59:11PM -0700, Nicolin Chen wrote:
I just realized that either my v8 or your version calls unmap() first at the entire cur_ioas. So, there seems to be no point in doing that fallback re-add routine since the cur_ioas isn't the same, which I don't feel quite right...
The point is to restore the access back to how it should be on failure so future use of the accesss still does the right thing.
We already have built into this a certain non-atomicity for mdevs, they can see a pin failure during replace if they race an access during this unmap window. This is similar to the real HW iommu's without atomic replace.
I was concerned about, after the replace, mdev losing all the mappings due to the unmap() call, which means the fallback is not really a status quo. Do you mean that they could pin those lost mappings back?
None of mdev drivers does that.
but we need think about the actual usage. I don't think the user can request ioas change w/o actually reconfiguring the mdev device. Presumably the latter could lead to reconstructure of pinned pages.
I can understand that the user should reconfigure the IOAS on success. Yet, should we expect it to reconfigure on a failure also?
I thought the user will likely stop the device before changing IOAS and then re-enable device DMA afterwards. If that is the typical flow then no matter this replace request succeeds or fails the re-enabling sequence should lead to the addition of pinned pages back to the current IOAS.
But this does imply inconsistent behavior between success and failure. Not sure whether it's worth a fix e.g. introducing another notifier for mdev drivers to re-pin...