On Fri, Jan 31, 2020 at 11:42:13AM +0000, Liu, Yi L wrote: > > I'm not very clear on the relationship betwen an IOMMUContext and a > > DualStageIOMMUObject. Can there be many IOMMUContexts to a > > DualStageIOMMUOBject? The other way around? Or is it just > > zero-or-one DualStageIOMMUObjects to an IOMMUContext? > > It is possible. As the below patch shows, DualStageIOMMUObject is per vfio > container. IOMMUContext can be either per-device or shared across devices, > it depends on vendor specific vIOMMU emulators. Is there an example when an IOMMUContext can be not per-device? It makes sense to me to have an object that is per-container (in your case, the DualStageIOMMUObject, IIUC), then we can connect that object to a device. However I'm a bit confused on why we've got two abstract layers (the other one is IOMMUContext)? That was previously for the whole SVA new APIs, now it's all moved over to the other new object, then IOMMUContext only register/unregister... Can we put the reg/unreg procedures into DualStageIOMMUObject as well? Then we drop the IOMMUContext (or say, keep IOMMUContext and drop DualStageIOMMUObject but let IOMMUContext to be per-vfio-container, the major difference is the naming here, say, PASID allocation does not seem to be related to dual-stage at all). Besides that, not sure I read it right... but even with your current series, the container->iommu_ctx will always only be bound to the first device created within that container, since you've got: group = vfio_get_group(groupid, pci_device_iommu_address_space(pdev), pci_device_iommu_context(pdev), errp); And: if (vfio_connect_container(group, as, iommu_ctx, errp)) { error_prepend(errp, "failed to setup container for group %d: ", groupid); goto close_fd_exit; } The iommu_ctx will be set to container->iommu_ctx if there's no existing container. > [RFC v3 10/25] vfio: register DualStageIOMMUObject to vIOMMU > https://www.spinics.net/lists/kvm/msg205198.html > > Take Intel vIOMMU as an example, there is a per device structure which > includes IOMMUContext instance and a DualStageIOMMUObject pointer. > > +struct VTDIOMMUContext { > + VTDBus *vtd_bus; > + uint8_t devfn; > + IOMMUContext iommu_context; > + DualStageIOMMUObject *dsi_obj; > + IntelIOMMUState *iommu_state; > +}; > https://www.spinics.net/lists/kvm/msg205196.html > > I think this would leave space for vendor specific vIOMMU emulators to > design their own relationship between an IOMMUContext and a > DualStageIOMMUObject. -- Peter Xu