[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: |
Alex Williamson |
Subject: |
Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications |
Date: |
Wed, 16 Nov 2016 08:34:42 -0700 |
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
- [Qemu-devel] [PATCH v6 3/3] IOMMU: enable intel_iommu map and unmap notifiers, (continued)
- [Qemu-devel] [PATCH v6 3/3] IOMMU: enable intel_iommu map and unmap notifiers, Aviv B.D, 2016/11/08
- [Qemu-devel] [PATCH v6 2/3] IOMMU: change iommu_op->translate's is_write to flags, add support to NO_FAIL flag mode, Aviv B.D, 2016/11/08
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Alex Williamson, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Alex Williamson, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Alex Williamson, 2016/11/10
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications,
Alex Williamson <=
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Aviv B.D., 2016/11/16
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Aviv B.D., 2016/11/21
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Alex Williamson, 2016/11/16
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Michael S. Tsirkin, 2016/11/16
- Re: [Qemu-devel] [PATCH v6 0/3] IOMMU: intel_iommu support map and unmap notifications, Aviv B.D., 2016/11/16