On Wed, 20 Jan 2021 03:05:49 +0000
"Tian, Kevin" <kevin.tian at intel.com> wrote:
> > From: Alex Williamson
> > Sent: Wednesday, January 20, 2021 8:51 AM
> >
> > On Wed, 20 Jan 2021 00:14:49 +0000
> > "Kasireddy, Vivek" <vivek.kasireddy at intel.com>
wrote:
> >
> > > Hi Alex,
> > >
> > > > -----Original Message-----
> > > > From: Alex Williamson <alex.williamson at redhat.com>
> > > > Sent: Tuesday, January 19, 2021 7:40 AM
> > > > To: Kasireddy, Vivek <vivek.kasireddy at intel.com>
> > > > Cc: virtualization at lists.linux-foundation.org; Kim,
Dongwon
> > <dongwon.kim at intel.com>
> > > > Subject: Re: [RFC 3/3] vfio: Share the KVM instance with
Vdmabuf
> > > >
> > > > On Tue, 19 Jan 2021 00:28:12 -0800
> > > > Vivek Kasireddy <vivek.kasireddy at intel.com> wrote:
> > > >
> > > > > Getting a copy of the KVM instance is necessary for
mapping Guest
> > > > > pages in the Host.
> > > > >
> > > > > TODO: Instead of invoking the symbol directly, there
needs to be a
> > > > > better way of getting a copy of the KVM instance
probably by using
> > > > > other notifiers. However, currently, KVM shares its
instance only
> > > > > with VFIO and therefore we are compelled to bind the
passthrough'd
> > > > > device to vfio-pci.
> > > >
> > > > Yeah, this is a bad solution, sorry, vfio is not going to
gratuitously
> > > > call out to vhost to share a kvm pointer. I'd prefer to
get rid of
> > > > vfio having any knowledge or visibility of the kvm pointer.
Thanks,
> > >
> > > [Kasireddy, Vivek] I agree that this is definitely not ideal as I
recognize it
> > > in the TODO. However, it looks like VFIO also gets a copy of the
KVM
> > > pointer in a similar manner:
> > >
> > > virt/kvm/vfio.c
> > >
> > > static void kvm_vfio_group_set_kvm(struct vfio_group *group,
struct kvm
> > *kvm)
> > > {
> > > void (*fn)(struct vfio_group *, struct kvm *);
> > >
> > > fn = symbol_get(vfio_group_set_kvm);
> > > if (!fn)
> > > return;
> > >
> > > fn(group, kvm);
> > >
> > > symbol_put(vfio_group_set_kvm);
> > > }
> >
> > You're equating the mechanism with the architecture. We use
symbols
> > here to avoid module dependencies between kvm and vfio, but this is
> > just propagating data that userspace is specifically registering
> > between kvm and vfio. vhost doesn't get to piggyback on that
channel.
> >
> > > With this patch, I am not suggesting that this is a precedent
that should be
> > followed
> > > but it appears there doesn't seem to be an alternative way of
getting a copy
> > of the KVM
> > > pointer that is clean and elegant -- unless I have not looked
hard enough. I
> > guess we
> > > could create a notifier chain with callbacks for VFIO and Vhost
that KVM
> > would call
> > > but this would mean modifying KVM.
> > >
> > > Also, if I understand correctly, if VFIO does not want to share
the KVM
> > pointer with
> > > VFIO groups, then I think it would break stuff like mdev which
counts on it.
> >
> > Only kvmgt requires the kvm pointer and the use case there is pretty
> > questionable, I wonder if it actually still exists now that we have
the
> > DMA r/w interface through vfio. Thanks,
> >
>
> IIRC, kvmgt still needs the kvm pointer to use kvm page tracking interface
> for write-protecting guest pgtable.
Thanks, Kevin. Either way, a vhost device has no stake in the game wrt
the kvm pointer lifecycle here and no business adding a callout. I'm
reluctant to add any further use cases even for mdevs as ideally mdevs
should have no dependency on kvm. Thanks,
Alex