Re: [PATCH v2 2/6] iommu/virtio: queue an UNMAP for every removed mapping

[sashiko-bot@kernel.org]

> viommu_unmap_pages() removed whole mappings from the tree and then, with
> mappings_lock dropped, queued a single UNMAP built from the range the
> caller asked for. Both halves are wrong in the direction the device's
> semantics are strict about: a device must not split a mapping, so the
> driver removes whole mappings - and an UNMAP whose range begins or ends
> inside another mapping invalidates nothing there, leaving the device
> mapping memory the driver has released; and between the removal and the
> queueing a concurrent map_pages() of the same range can put its MAP in
> front of the UNMAP.
> 
> Queue the UNMAPs in the critical section that removes the mappings, one
> request per contiguous run of removed mappings, covering exactly the
> mappings' own ranges: a run's mappings are removed together and its
> request covers all of them, nothing else. If a request cannot be queued,
> the mapping being examined stays in the tree and the walk stops there,
> and the run whose request could not be queued is not reported as
> unmapped. That is not an atomic rollback: the run's mappings were already
> removed from the tree and freed, and the caller is told the bytes that
> really were unmapped, not that the whole range failed to unmap. The
> request itself is still allocated on the unmap path here; the next patch
> moves that allocation to the map path, where it is allowed to fail.
> 
> Fixes: edcd69ab9a32 ("iommu: Add virtio-iommu driver")
> Signed-off-by: Anlai Lu <agicy@qq.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_D757B544955DDC509B5DC7161F425620BA08@qq.com?part=2