qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap


From: Aviv B.D.
Subject: Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications
Date: Mon, 21 Nov 2016 13:36:21 +0200

On Wed, Nov 16, 2016 at 9:57 PM, Michael S. Tsirkin <address@hidden> wrote:

> On Wed, Nov 16, 2016 at 09:50:46PM +0200, Aviv B.D. wrote:
> >
> >
> > On Wed, Nov 16, 2016 at 5:34 PM, Alex Williamson <
> address@hidden>
> > wrote:
> >
> >     On Wed, 16 Nov 2016 15:54:56 +0200
> >     "Michael S. Tsirkin" <address@hidden> wrote:
> >
> >     > On Thu, Nov 10, 2016 at 12:44:47PM -0700, Alex Williamson wrote:
> >     > > On Thu, 10 Nov 2016 21:20:36 +0200
> >     > > "Michael S. Tsirkin" <address@hidden> wrote:
> >     > >
> >     > > > On Thu, Nov 10, 2016 at 09:04:13AM -0700, Alex Williamson
> wrote:
> >     > > > > On Thu, 10 Nov 2016 17:54:35 +0200
> >     > > > > "Michael S. Tsirkin" <address@hidden> wrote:
> >     > > > >
> >     > > > > > On Thu, Nov 10, 2016 at 08:30:21AM -0700, Alex Williamson
> wrote:
> >     > > > > > > On Thu, 10 Nov 2016 17:14:24 +0200
> >     > > > > > > "Michael S. Tsirkin" <address@hidden> wrote:
> >     > > > > > >
> >     > > > > > > > On Tue, Nov 08, 2016 at 01:04:21PM +0200, Aviv B.D
> wrote:
> >     > > > > > > > > From: "Aviv Ben-David" <address@hidden>
> >     > > > > > > > >
> >     > > > > > > > > * Advertize Cache Mode capability in iommu cap
> register.
> >     > > > > > > > >   This capability is controlled by "cache-mode"
> property of
> >     intel-iommu device.
> >     > > > > > > > >   To enable this option call QEMU with "-device
> >     intel-iommu,cache-mode=true".
> >     > > > > > > > >
> >     > > > > > > > > * On page cache invalidation in intel vIOMMU, check
> if the
> >     domain belong to
> >     > > > > > > > >   registered notifier, and notify accordingly.
> >     > > > > > > >
> >     > > > > > > > This looks sane I think. Alex, care to comment?
> >     > > > > > > > Merging will have to wait until after the release.
> >     > > > > > > > Pls remember to re-test and re-ping then.
> >     > > > > > >
> >     > > > > > > I don't think it's suitable for upstream until there's a
> >     reasonable
> >     > > > > > > replay mechanism
> >     > > > > >
> >     > > > > > Could you pls clarify what do you mean by replay?
> >     > > > > > Is this when you attach a device by hotplug to
> >     > > > > > a running system?
> >     > > > > >
> >     > > > > > If yes this can maybe be addressed by disabling hotplug
> >     temporarily.
> >     > > > >
> >     > > > > No, hotplug is not required, moving a device between existing
> >     domains
> >     > > > > requires replay, ie. actually using it for nested device
> >     assignment.
> >     > > >
> >     > > > Good point, that one is a correctness thing. Aviv,
> >     > > > could you add this in TODO list in a cover letter pls?
> >     > > >
> >     > > > > > > and we straighten out whether it's expected to get
> >     > > > > > > multiple notifies and the notif-ee is responsible for
> filtering
> >     > > > > > > them or if the notif-er should do filtering.
> >     > > > > >
> >     > > > > > OK this is a documentation thing.
> >     > > > >
> >     > > > > Well no, it needs to be decided and if necessary implemented.
> >     > > >
> >     > > > Let's assume it's the notif-ee for now. Less is more and all
> that.
> >     > >
> >     > > I think this is opposite of the approach dwg suggested.
> >     > >
> >     > > > > > >  Without those, this is
> >     > > > > > > effectively just an RFC.
> >     > > > > >
> >     > > > > > It's infrastructure without users so it doesn't break
> things,
> >     > > > > > I'm more interested in seeing whether it's broken in
> >     > > > > > some way than whether it's complete.
> >     > > > >
> >     > > > > If it allows use with vfio but doesn't fully implement the
> complete
> >     set
> >     > > > > of interfaces, it does break things.  We currently prevent
> viommu
> >     usage
> >     > > > > with vfio because it is incomplete.
> >     > > >
> >     > > > Right - that bit is still in as far as I can see.
> >     > >
> >     > > Nope, 3/3 changes vtd_iommu_notify_flag_changed() to allow use
> with
> >     > > vfio even though it's still incomplete.  We would at least need
> >     > > something like a replay callback for VT-d that triggers an abort
> if you
> >     > > still want to accept it incomplete.  Thanks,
> >     > >
> >     > > Alex
> >     >
> >     > IIUC practically things seems to work, right?
> >
> >     AFAIK, no.
> >
> >     > So how about disabling by default with a flag for people that want
> to
> >     > experiment with it?
> >     > E.g. x-vfio-allow-broken-translations ?
> >
> >     We've already been through one round of "intel-iommu is incomplete
> for
> >     use with device assignment, how can we prevent it from being used",
> >     which led to the notify_flag_changed callback on
> MemoryRegionIOMMUOps.
> >     This series now claims to fix that yet still doesn't provide a
> >     mechanism to do memory_region_iommu_replay() given that VT-d has a
> much
> >     larger address width.  Why is the onus on vfio to resolve this or
> >     provide some sort of workaround?  vfio is using the QEMU iommu
> >     interface correctly, intel-iommu is still incomplete. The least it
> >     could do is add an optional replay callback to MemoryRegionIOMMUOps
> >     that supersedes the existing memory_region_iommu_replay() code and
> >     triggers an abort when it gets called.  I don't know what an
> >     x-vfio-allow-broken-translations option would do, how I'd implement
> it,
> >     or why I'd bother to implement it.  Thanks,
> >
> >     Alex
> >
> >
> > I'll implement your suggestion regarding the replay framwork.
> > Probably in another patch set, if it is OK?
> >
> > Thanks,
> > Aviv.
> >
>
> Alex points out that with 3/3 setting cache_mode=1 will enable VFIO
> device assignment. Question would be, does it actually work for you
> in this mode? If not it seems important not to enable it for users.
>
> --
> MST
>

The patch is actually working with just commenting out the replay call in
vfio.
I didn't tested it with nesting environment but with DPDK it worked great.

Aviv.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]