| Message ID | 1479794973-11910-4-git-send-email-jike.song@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, 22 Nov 2016 14:09:33 +0800 Jike Song <jike.song@intel.com> wrote: > Sometimes users need to be aware when a vfio_group attaches to a > KVM or detaches from it. KVM already calls get/put method from vfio to > manipulate the vfio_group reference, it can notify vfio_group in > a similar way. > > Cc: Alex Williamson <alex.williamson@redhat.com> > Cc: Kirti Wankhede <kwankhede@nvidia.com> > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> > Signed-off-by: Jike Song <jike.song@intel.com> > --- > virt/kvm/vfio.c | 31 +++++++++++++++++++++++++++++++ > 1 file changed, 31 insertions(+) > > diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c > index 1dd087d..ea5b4bb 100644 > --- a/virt/kvm/vfio.c > +++ b/virt/kvm/vfio.c > @@ -60,6 +60,32 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group) > symbol_put(vfio_group_put_external_user); > } > > +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); > +} > + > +static void kvm_vfio_group_clear_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, NULL); > + > + symbol_put(vfio_group_set_kvm); > +} Why do we need this function? Can't we simply call kvm_vfio_group_set_kvm(vfio_group, NULL)? Not to mention that the struct kvm arg is unused here otherwise. > + > static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group) > { > long (*fn)(struct vfio_group *, unsigned long); > @@ -155,6 +181,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) > list_add_tail(&kvg->node, &kv->group_list); > kvg->vfio_group = vfio_group; > > + kvm_vfio_group_set_kvm(vfio_group, dev->kvm); > + > kvm_arch_start_assignment(dev->kvm); > > mutex_unlock(&kv->lock); > @@ -196,6 +224,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) > > mutex_unlock(&kv->lock); > > + kvm_vfio_group_clear_kvm(vfio_group, dev->kvm); > + Why did we set_kvm within kv->lock, but clear it outside of kv->lock? I'm not sure kv->lock is particularly relevant to us but we might as well be consistent. > kvm_vfio_group_put_external_user(vfio_group); > > kvm_vfio_update_coherency(dev); > @@ -240,6 +270,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev) > struct kvm_vfio_group *kvg, *tmp; > > list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) { > + kvm_vfio_group_clear_kvm(kvg->vfio_group, dev->kvm); > kvm_vfio_group_put_external_user(kvg->vfio_group); > list_del(&kvg->node); > kfree(kvg); -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 12/01/2016 01:02 AM, Alex Williamson wrote: > On Tue, 22 Nov 2016 14:09:33 +0800 > Jike Song <jike.song@intel.com> wrote: > >> Sometimes users need to be aware when a vfio_group attaches to a >> KVM or detaches from it. KVM already calls get/put method from vfio to >> manipulate the vfio_group reference, it can notify vfio_group in >> a similar way. >> >> Cc: Alex Williamson <alex.williamson@redhat.com> >> Cc: Kirti Wankhede <kwankhede@nvidia.com> >> Cc: Paolo Bonzini <pbonzini@redhat.com> >> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> >> Signed-off-by: Jike Song <jike.song@intel.com> >> --- >> virt/kvm/vfio.c | 31 +++++++++++++++++++++++++++++++ >> 1 file changed, 31 insertions(+) >> >> diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c >> index 1dd087d..ea5b4bb 100644 >> --- a/virt/kvm/vfio.c >> +++ b/virt/kvm/vfio.c >> @@ -60,6 +60,32 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group) >> symbol_put(vfio_group_put_external_user); >> } >> >> +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); >> +} >> + >> +static void kvm_vfio_group_clear_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, NULL); >> + >> + symbol_put(vfio_group_set_kvm); >> +} > > Why do we need this function? Can't we simply call > kvm_vfio_group_set_kvm(vfio_group, NULL)? Not to mention that the > struct kvm arg is unused here otherwise. This was the place to call kvm_put_kvm in previous series, but yes, it's not necessary in current series, will drop it. >> + >> static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group) >> { >> long (*fn)(struct vfio_group *, unsigned long); >> @@ -155,6 +181,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) >> list_add_tail(&kvg->node, &kv->group_list); >> kvg->vfio_group = vfio_group; >> >> + kvm_vfio_group_set_kvm(vfio_group, dev->kvm); >> + >> kvm_arch_start_assignment(dev->kvm); >> >> mutex_unlock(&kv->lock); >> @@ -196,6 +224,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) >> >> mutex_unlock(&kv->lock); >> >> + kvm_vfio_group_clear_kvm(vfio_group, dev->kvm); >> + > > > Why did we set_kvm within kv->lock, but clear it outside of kv->lock? > I'm not sure kv->lock is particularly relevant to us but we might as > well be consistent. kv->lock is not to protect the vfio_group anyway, so being out of the lock should be safe. But placing set_kvm before acquiring kv->lock will generate more code in error path, so I guess the better way is to place it after unlocking. Will reflect that in next version. >> kvm_vfio_group_put_external_user(vfio_group); >> >> kvm_vfio_update_coherency(dev); >> @@ -240,6 +270,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev) >> struct kvm_vfio_group *kvg, *tmp; >> >> list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) { >> + kvm_vfio_group_clear_kvm(kvg->vfio_group, dev->kvm); >> kvm_vfio_group_put_external_user(kvg->vfio_group); >> list_del(&kvg->node); >> kfree(kvg); > -- Thanks, Jike -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/virt/kvm/vfio.c b/virt/kvm/vfio.c index 1dd087d..ea5b4bb 100644 --- a/virt/kvm/vfio.c +++ b/virt/kvm/vfio.c @@ -60,6 +60,32 @@ static void kvm_vfio_group_put_external_user(struct vfio_group *vfio_group) symbol_put(vfio_group_put_external_user); } +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); +} + +static void kvm_vfio_group_clear_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, NULL); + + symbol_put(vfio_group_set_kvm); +} + static bool kvm_vfio_group_is_coherent(struct vfio_group *vfio_group) { long (*fn)(struct vfio_group *, unsigned long); @@ -155,6 +181,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) list_add_tail(&kvg->node, &kv->group_list); kvg->vfio_group = vfio_group; + kvm_vfio_group_set_kvm(vfio_group, dev->kvm); + kvm_arch_start_assignment(dev->kvm); mutex_unlock(&kv->lock); @@ -196,6 +224,8 @@ static int kvm_vfio_set_group(struct kvm_device *dev, long attr, u64 arg) mutex_unlock(&kv->lock); + kvm_vfio_group_clear_kvm(vfio_group, dev->kvm); + kvm_vfio_group_put_external_user(vfio_group); kvm_vfio_update_coherency(dev); @@ -240,6 +270,7 @@ static void kvm_vfio_destroy(struct kvm_device *dev) struct kvm_vfio_group *kvg, *tmp; list_for_each_entry_safe(kvg, tmp, &kv->group_list, node) { + kvm_vfio_group_clear_kvm(kvg->vfio_group, dev->kvm); kvm_vfio_group_put_external_user(kvg->vfio_group); list_del(&kvg->node); kfree(kvg);
Sometimes users need to be aware when a vfio_group attaches to a KVM or detaches from it. KVM already calls get/put method from vfio to manipulate the vfio_group reference, it can notify vfio_group in a similar way. Cc: Alex Williamson <alex.williamson@redhat.com> Cc: Kirti Wankhede <kwankhede@nvidia.com> Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Xiao Guangrong <guangrong.xiao@linux.intel.com> Signed-off-by: Jike Song <jike.song@intel.com> --- virt/kvm/vfio.c | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+)