Re: [Qemu-devel] [PATCH v7 3/5] IOMMU: enable intel_iommu map and unmap notifiers

[Peter Xu]

On Mon, Dec 19, 2016 at 11:53:32AM +0000, Liu, Yi L wrote:

[...]

> > > Regards to the s->notifiers_list, I didn't see the init op to it. Does it 
> > > happen
> > > in another patch? If so, it may be better to move it in this patch since 
> > > this
> > > patch introduces both the definition and usage of notifiers_list.
> > >
> > > If it is already clarified, then ignore it.
> > 
> > I think it was missing. It IMHO accidentally worked since QLIST_INIT()
> > just set the head to NULL and that's what we did when we create the
> > IntelIOMMUState object.
> > 
> > And what's worse - I found this approach may not work if we do
> > QLIST_INSERT() in the changed() hook, since if we have more than one
> > assigned devices we will only register the first one not the rest. A
> > better approach may be traversing the vt-d buses via
> > IntelIOMMUState.vtd_as_by_busptr.
> > 
> Peter,
> 
> In Oct, I also mailed Aviv about using IntelIOMMUState.vtd_as_by_busptr
> when trying to connect the vfio notifiers(map/unmap) and vIOMMU. 
> However, I reconsidered it later. If I remember correctly, 
> IntelIOMMUState.vtd_as_by_busptr not only includes vtd_as for assigned 
> devices,
> but also includes virtual devices. When iotlb invalidation comes to vIOMMU, 
> there
> is no indication for which device in iotlb_inv_desc. So still need to have a 
> list to record
> vtd_as which needs to be looped. So I keep silent on it after that thought.
> 
> Now, you mentioned it may not work in multi-assigned scenario. Maybe it's
> time to reconsider it again. 

Hmm, first parameter of vtd_iommu_notify_flag_changed() is memory
region, and that's per-device. So current approach should work even
with multiple devices. Looks like I made a mistake, sorry. :) 

Thanks,

-- peterx