Re: [RFC v3 03/25] hw/iommu: introduce IOMMUContext

[Peter Xu <peterx@redhat.com>]

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