[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT)
From: |
Peter Xu |
Subject: |
Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT) |
Date: |
Thu, 20 Apr 2017 11:04:27 +0800 |
User-agent: |
Mutt/1.5.24 (2015-08-30) |
On Wed, Apr 19, 2017 at 03:27:52PM +0800, Lan Tianyu wrote:
> Hi All:
> Sorry for later response.
> On 2017年04月18日 17:04, Liu, Yi L wrote:
> >> -----Original Message-----
> >> From: Peter Xu [mailto:address@hidden
> >> Sent: Tuesday, April 18, 2017 3:27 PM
> >> To: Liu, Yi L <address@hidden>
> >> Cc: address@hidden; Lan, Tianyu <address@hidden>; Michael S .
> >> Tsirkin <address@hidden>; Jason Wang <address@hidden>; Marcel
> >> Apfelbaum <address@hidden>; David Gibson <address@hidden>
> >> Subject: Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough
> >> (PT)
> >>
> >> On Tue, Apr 18, 2017 at 06:02:44AM +0000, Liu, Yi L wrote:
> >>>>> Hi Peter,
> >>>>>
> >>>>> Skip address space switching is a good idea to support Passthru mode.
> >>>>> However, without the address space, the vfio notifier would not be
> >>>>> registered, thus vIOMMU emulator has no way to connect to host. It
> >>>>> is no harm if there is only map/unmap notifier. But if we have
> >>>>> more notifiers other than map/unmap, it may be a problem.
> >>>>>
> >>>>> I think we need to reconsider it here.
> >>>>
> >>>> For now I think as switching is good to us in general. Could I know
> >>>> more context about this? Would it be okay to work on top of this in the
> >>>> future?
> >>>>
> >>>
> >>> Let me explain. For vSVM enabling, it needs to add new notifier to
> >>> bind gPASID table to host. If an assigned device uses SVM in guest, as
> >>> designed guest IOMMU driver would set "pt" for it. This is to make
> >>> sure the host second-level page table stores GPA->HPA mapping. So that
> >>> pIOMMU can do nested translation to achieve gVA->GPA GPA->HPA mapping.
> >>
> >> That should mean that in the guest it will only enable first level
> >> translation, while in
> >> the host it'll be nested (first + second)? Then you should be trapping the
> >> guest
> >> extended context entry invalidation, then pass the PASID table pointer
> >> downward to
> >> the host IOMMU, am I correct?
> >
> > exactly.
> >
> >>
> >>>
> >>> So the situation is if we want to keep GPA->HPA mapping, we should
> >>> skip address space switch, but the vfio listener would not know vIOMMU
> >>> is added, so no vfio notifier would be registered. But if we do the
> >>> switch, the GPA->HPA mapping is unmapped. And need another way to build
> >>> the
> >> GPA->HPA mapping.
> >>
> >> If my understanding above is correct, current IOMMU notifier may not
> >> satisfy your
> >> need. If you see the current notify interface, it's delivering
> >> IOMMUTLBEntry. I
> >> suspect it only suites for IOTLB notifications, but not if you want to
> >> pass some
> >> pointer (one GPA) downward somewhere.
> >
> > Yeah, you got it more than absolutely. One of my patch is to modify the
> > para to be
> > void * and let each notifiers to translate separately. I think it should be
> > a reasonable
> > change.
> >
> > Supposedly, I would send RFC for vSVM soon. We may talk more it at that
> > thread.
I suspect whether it'll be good that we hang everything under current
IOMMU notifiers... But sure I'm looking forward to your RFC. :)
> >
> >>>
> >>> I think we may have two choice here. Pls let me know your ideas.
> >>>
> >>> 1. skip the switch for "pt" mode, find other way to register vfio
> >>> notifiers. not sure if this is workable. Just a quick thought.
> >>>
> >>> 2. do the switch, and rebuild GPA->HPA mapping if device use "pt"
> >>> mode. For this option, I also have two way to build the GPA->HPA mapping.
> >>> a) walk all the memory region sections in address_space_memory, and build
> >>> the
> >> mapping.
> >>> address_space_memory.dispatch->map.sections, sections stores all the
> >>> mapped
> >> sections.
> >>>
> >>> b) let guest iommu driver mimics a 1:1 mapping for the devices use
> >>> "pt" mode. in this way, the GPA->HPA mapping could be setup by walking
> >>> the guest SL page table. This is consistent with your vIOVA replay
> >>> solution.
> >>
> >> I'll prefer (1). Reason explained above.
> >>
> >>>
> >>> Also I'm curious if Tianyu's fault report framework needs to register new
> >>> notifiers.
> >>
> >> For Tianyu's case, I feel like we need other kind of notifier as well, but
> >> not the
> >> current IOTLB one. And, of course in this case it'll be in an reverted
> >> direction, which
> >> is from device to vIOMMU.
> >>
> >> (I am thinking whether it's a good time now to let any PCI device able to
> >> fetch its
> >> direct owner IOMMU object, then it can register anything it want onto
> >> that object
> >> maybe?)
> >>
> > Hmmm, let's see Tianyu's comment as he's driving fault report. Let's keep
> > in touch on
> > this Passthru-Mode enabling.
>
> In my previous RFC patchset of fault event reporting, I registered fault
> notifier when there is a VFIO group attached to VFIO container and used
> the address space to check whether vIOMMU is added. The address space is
> returned by vtd_host_dma_iommu(). vtd_find_add_as() initializes device's
> IOMMU memory region and put it into device's address space as root
> memory region.
> Does this make sense?
>
> @@ -1103,6 +1132,14 @@ static int vfio_connect_container(VFIOGroup
> *group, AddressSpace *as,
> goto listener_release_exit;
> }
>
> + if (memory_region_is_iommu(container->space->as->root)) {
I would suggest we don't play with as->root any more. After vtd vfio
series, this may not be true if passthrough mode is enabled (then the
root may be switched to an system memory alias). I don't know the best
way to check this, one alternative might be that we check whether
container->space->as == system_memory(), it should be workable, but in
a slightly hackish way.
Thanks,
> + if (vfio_set_iommu_fault_notifier(container)) {
> + error_setg_errno(errp, -ret,
> + "Fail to set IOMMU fault notifier");
> + goto listener_release_exit;
> + }
> + }
> +
> container->initialized = true;
>
> --
> Best regards
> Tianyu Lan
--
Peter Xu
- [Qemu-devel] [PATCH v2 4/7] intel_iommu: renaming context entry helpers, (continued)
- [Qemu-devel] [PATCH v2 4/7] intel_iommu: renaming context entry helpers, Peter Xu, 2017/04/17
- [Qemu-devel] [PATCH v2 5/7] intel_iommu: provide vtd_ce_get_type(), Peter Xu, 2017/04/17
- [Qemu-devel] [PATCH v2 6/7] intel_iommu: use IOMMU_ACCESS_FLAG(), Peter Xu, 2017/04/17
- [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/17
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/18
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/18
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/18
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/18
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/18
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Lan Tianyu, 2017/04/19
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT),
Peter Xu <=
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Lan Tianyu, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Lan, Tianyu, 2017/04/20
- Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Peter Xu, 2017/04/20
Re: [Qemu-devel] [PATCH v2 7/7] intel_iommu: support passthrough (PT), Liu, Yi L, 2017/04/19
Re: [Qemu-devel] [PATCH v2 0/7] VT-d: PT (passthrough) mode support and misc fixes, Paolo Bonzini, 2017/04/18