lkml.org 
[lkml]   [2013]   [Jun]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 3/4] KVM: PPC: Add support for IOMMU in-kernel handling
    On Thu, Jun 20, 2013 at 02:58:18PM +1000, Alexey Kardashevskiy wrote:
    > On 06/20/2013 01:49 AM, Alex Williamson wrote:
    > > On Thu, 2013-06-20 at 00:50 +1000, Benjamin Herrenschmidt wrote:
    > >> On Wed, 2013-06-19 at 11:58 +0200, Alexander Graf wrote:
    > >>
    > >>>> Alex, any objection ?
    > >>>
    > >>> Which Alex? :)
    > >>
    > >> Heh, mostly Williamson in this specific case but your input is still
    > >> welcome :-)
    > >>
    > >>> I think validate works, it keeps iteration logic out of the kernel
    > >>> which is a good thing. There still needs to be an interface for
    > >>> getting the iommu id in VFIO, but I suppose that one's for the other
    > >>> Alex and Jörg to comment on.
    > >>
    > >> I think getting the iommu fd is already covered by separate patches from
    > >> Alexey.
    > >>
    > >>>>
    > >>>> Do we need to make it a get/put interface instead ?
    > >>>>
    > >>>> vfio_validate_and_use_iommu(file, iommu_id);
    > >>>>
    > >>>> vfio_release_iommu(file, iommu_id);
    > >>>>
    > >>>> To ensure that the resource remains owned by the process until KVM
    > >>>> is closed as well ?
    > >>>>
    > >>>> Or do we want to register with VFIO with a callback so that VFIO can
    > >>>> call us if it needs us to give it up ?
    > >>>
    > >>> Can't we just register a handler on the fd and get notified when it
    > >>> closes? Can you kill VFIO access without closing the fd?
    > >>
    > >> That sounds actually harder :-)
    > >>
    > >> The question is basically: When we validate that relationship between a
    > >> specific VFIO struct file with an iommu, what is the lifetime of that
    > >> and how do we handle this lifetime properly.
    > >>
    > >> There's two ways for that sort of situation: The notification model
    > >> where we get notified when the relationship is broken, and the refcount
    > >> model where we become a "user" and thus delay the breaking of the
    > >> relationship until we have been disposed of as well.
    > >>
    > >> In this specific case, it's hard to tell what is the right model from my
    > >> perspective, which is why I would welcome Alex (W.) input.
    > >>
    > >> In the end, the solution will end up being in the form of APIs exposed
    > >> by VFIO for use by KVM (via that symbol lookup mechanism) so Alex (W),
    > >> as owner of VFIO at this stage, what do you want those to look
    > >> like ? :-)
    > >
    > > My first thought is that we should use the same reference counting as we
    > > have for vfio devices (group->container_users). An interface for that
    > > might look like:
    > >
    > > int vfio_group_add_external_user(struct file *filep)
    > > {
    > > struct vfio_group *group = filep->private_data;
    > >
    > > if (filep->f_op != &vfio_group_fops)
    > > return -EINVAL;
    > >
    > >
    > > if (!atomic_inc_not_zero(&group->container_users))
    > > return -EINVAL;
    > >
    > > return 0;
    > > }
    > >
    > > void vfio_group_del_external_user(struct file *filep)
    > > {
    > > struct vfio_group *group = filep->private_data;
    > >
    > > BUG_ON(filep->f_op != &vfio_group_fops);
    > >
    > > vfio_group_try_dissolve_container(group);
    > > }
    > >
    > > int vfio_group_iommu_id_from_file(struct file *filep)
    > > {
    > > struct vfio_group *group = filep->private_data;
    > >
    > > BUG_ON(filep->f_op != &vfio_group_fops);
    > >
    > > return iommu_group_id(group->iommu_group);
    > > }
    > >
    > > Would that work? Thanks,
    >
    >
    > Just out of curiosity - would not get_file() and fput_atomic() on a group's
    > file* do the right job instead of vfio_group_add_external_user() and
    > vfio_group_del_external_user()?

    I was thinking that too. Grabbing a file reference would certainly be
    the usual way of handling this sort of thing.

    --
    David Gibson | I'll have my music baroque, and my code
    david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_
    | _way_ _around_!
    http://www.ozlabs.org/~dgibson
    [unhandled content-type:application/pgp-signature]
    \
     
     \ /
      Last update: 2013-06-20 09:41    [W:4.956 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site