[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: |
Thu, 10 Nov 2016 12:44:47 -0700 |
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
> > > The patchset spent out of tree too long and I'd like to see
> > > us make progress towards device assignment working with
> > > vIOMMU sooner rather than later, so if it's broken I won't
> > > merge it but if it's incomplete I will.
> >
> > So long as it's incomplete and still prevents vfio usage, I'm ok with
> > merging it, but I don't want to enable vfio usage until it's complete.
> > Thanks,
> >
> > Alex
> >
> > > > > > Currently this patch still doesn't enabling VFIO devices support
> > > > > > with vIOMMU
> > > > > > present. Current problems:
> > > > > > * vfio_iommu_map_notify is not aware about memory range belong to
> > > > > > specific
> > > > > > VFIOGuestIOMMU.
> > > > > > * memory_region_iommu_replay hangs QEMU on start up while it
> > > > > > itterate over
> > > > > > 64bit address space. Commenting out the call to this function
> > > > > > enables
> > > > > > workable VFIO device while vIOMMU present.
> > > > > > * vfio_iommu_map_notify should check if address space range is
> > > > > > suitable for
> > > > > > current notifier.
> > > > > >
> > > > > > Changes from v1 to v2:
> > > > > > * remove assumption that the cache do not clears
> > > > > > * fix lockup on high load.
> > > > > >
> > > > > > Changes from v2 to v3:
> > > > > > * remove debug leftovers
> > > > > > * split to sepearate commits
> > > > > > * change is_write to flags in vtd_do_iommu_translate, add
> > > > > > IOMMU_NO_FAIL
> > > > > > to suppress error propagating to guest.
> > > > > >
> > > > > > Changes from v3 to v4:
> > > > > > * Add property to intel_iommu device to control the CM capability,
> > > > > > default to False.
> > > > > > * Use s->iommu_ops.notify_flag_changed to register notifiers.
> > > > > >
> > > > > > Changes from v4 to v4 RESEND:
> > > > > > * Fix codding style pointed by checkpatch.pl script.
> > > > > >
> > > > > > Changes from v4 to v5:
> > > > > > * Reduce the number of changes in patch 2 and make flags real
> > > > > > bitfield.
> > > > > > * Revert deleted debug prints.
> > > > > > * Fix memory leak in patch 3.
> > > > > >
> > > > > > Changes from v5 to v6:
> > > > > > * fix prototype of iommu_translate function for more IOMMU types.
> > > > > > * VFIO will be notified only on the difference, without unmap
> > > > > > before change to maps.
> > > > > >
> > > > > > Aviv Ben-David (3):
> > > > > > IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility
> > > > > > exposoed to
> > > > > > guest
> > > > > > IOMMU: change iommu_op->translate's is_write to flags, add
> > > > > > support to
> > > > > > NO_FAIL flag mode
> > > > > > IOMMU: enable intel_iommu map and unmap notifiers
> > > > > >
> > > > > > exec.c | 3 +-
> > > > > > hw/alpha/typhoon.c | 2 +-
> > > > > > hw/i386/amd_iommu.c | 4 +-
> > > > > > hw/i386/intel_iommu.c | 160
> > > > > > +++++++++++++++++++++++++++++++++--------
> > > > > > hw/i386/intel_iommu_internal.h | 3 +
> > > > > > hw/pci-host/apb.c | 2 +-
> > > > > > hw/ppc/spapr_iommu.c | 2 +-
> > > > > > hw/s390x/s390-pci-bus.c | 2 +-
> > > > > > include/exec/memory.h | 6 +-
> > > > > > include/hw/i386/intel_iommu.h | 11 +++
> > > > > > memory.c | 3 +-
> > > > > > 11 files changed, 160 insertions(+), 38 deletions(-)
> > > > > >
> > > > > > --
> > > > > > 1.9.1
> > > > >
- Re: [Qemu-devel] [PATCH v6 1/3] IOMMU: add option to enable VTD_CAP_CM to vIOMMU capility exposoed to guest, (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 <=
- 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, 2016/11/16
- 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