From: "Michael S. Tsirkin" <mst@redhat.com>
To: Peter Xu <peterx@redhat.com>
Cc: Eric Auger <eric.auger@redhat.com>,
eric.auger.pro@gmail.com, qemu-arm@nongnu.org,
qemu-devel@nongnu.org, jean-philippe@linaro.org,
bharat.bhushan@nxp.com, alex.williamson@redhat.com
Subject: Re: [PATCH for 8.0 0/2] virtio-iommu: Fix Replay
Date: Tue, 20 Dec 2022 09:59:27 -0500 [thread overview]
Message-ID: <20221220095908-mutt-send-email-mst@kernel.org> (raw)
In-Reply-To: <Y5H7fu2ikdXU8b3i@x1n>
On Thu, Dec 08, 2022 at 09:58:06AM -0500, Peter Xu wrote:
> On Thu, Dec 08, 2022 at 08:48:09AM +0100, Eric Auger wrote:
> > Hi Peter,
>
> Hi, Eric,
>
> >
> > On 12/8/22 00:49, Peter Xu wrote:
> > > Hi, Eric,
> > >
> > > On Wed, Dec 07, 2022 at 02:36:44PM +0100, Eric Auger wrote:
> > >> When assigning VFIO devices protected by a virtio-iommu we need to replay
> > >> the mappings when adding a new IOMMU MR and when attaching a device to
> > >> a domain. While we do a "remap" we currently fail to first unmap the
> > >> existing IOVA mapping and just map the new one. With some device/group
> > >> topology this can lead to errors in VFIO when trying to DMA_MAP IOVA
> > >> ranges onto existing ones.
> > > I'm not sure whether virtio-iommu+vfio will suffer from DMA races like when
> > > we were working on the vt-d replay for vfio. The issue is whether DMA can
> > > happen right after UNMAP but before MAP of the same page if the page was
> > > always mapped.
> >
> > I don't think it can race because a mutex is hold while doing the
> > virtio_iommu_replay(), and each time a virtio cmd is handled (attach,
> > map, unmap), see virtio_iommu_handle_command.
> > So I think it is safe.
>
> It's not the race in the code, it's the race between modifying host IOMMU
> pgtable with DMA happening in parallel. The bug triggered with DMA_MAP
> returning -EEXIST means there's existing mapping.
>
> If during replay there's mapped ranges and the ranges are prone to DMA,
> then IIUC it can happen.
>
> I didn't really check specifically for virtio-iommu and I mostly forget the
> details, just to raise this up. It's possible for some reason it just
> can't trigger. VT-d definitely can, in which case we'll see DMA errors on
> the host from the assigned device when the DMA triggers during the "unmap
> and map" window.
>
> Thanks,
Eric any resolution on this?
> --
> Peter Xu
next prev parent reply other threads:[~2022-12-20 14:59 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-07 13:36 [PATCH for 8.0 0/2] virtio-iommu: Fix Replay Eric Auger
2022-12-07 13:36 ` [PATCH for 8.0 1/2] virtio-iommu: Add unmap on virtio_iommu_remap() Eric Auger
2022-12-07 13:36 ` [PATCH for 8.0 2/2] virtio-iommu: Fix replay on device attach Eric Auger
2022-12-07 23:49 ` [PATCH for 8.0 0/2] virtio-iommu: Fix Replay Peter Xu
2022-12-08 7:48 ` Eric Auger
2022-12-08 14:58 ` Peter Xu
2022-12-20 14:59 ` Michael S. Tsirkin [this message]
2022-12-20 15:06 ` Eric Auger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20221220095908-mutt-send-email-mst@kernel.org \
--to=mst@redhat.com \
--cc=alex.williamson@redhat.com \
--cc=bharat.bhushan@nxp.com \
--cc=eric.auger.pro@gmail.com \
--cc=eric.auger@redhat.com \
--cc=jean-philippe@linaro.org \
--cc=peterx@redhat.com \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.