On Thu, Mar 16, 2023 at 9:31 AM Stefano Garzarella <sgarz...@redhat.com> wrote:
>
> On Tue, Mar 14, 2023 at 11:48:33AM +0800, Jason Wang wrote:
> >On Thu, Mar 2, 2023 at 7:34 PM Stefano Garzarella <sgarz...@redhat.com>
> >wrote:
> >>
> >> When the user call VHOST_SET_OWNER ioctl and the vDPA device
> >> has `use_va` set to true, let's call the bind_mm callback.
> >> In this way we can bind the device to the user address space
> >> and directly use the user VA.
> >>
> >> The unbind_mm callback is called during the release after
> >> stopping the device.
> >>
> >> Signed-off-by: Stefano Garzarella <sgarz...@redhat.com>
> >> ---
> >>
> >> Notes:
> >> v2:
> >> - call the new unbind_mm callback during the release [Jason]
> >> - avoid to call bind_mm callback after the reset, since the device
> >> is not detaching it now during the reset
> >>
> >> drivers/vhost/vdpa.c | 30 ++++++++++++++++++++++++++++++
> >> 1 file changed, 30 insertions(+)
> >>
> >> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
> >> index dc12dbd5b43b..1ab89fccd825 100644
> >> --- a/drivers/vhost/vdpa.c
> >> +++ b/drivers/vhost/vdpa.c
> >> @@ -219,6 +219,28 @@ static int vhost_vdpa_reset(struct vhost_vdpa *v)
> >> return vdpa_reset(vdpa);
> >> }
> >>
> >> +static long vhost_vdpa_bind_mm(struct vhost_vdpa *v)
> >> +{
> >> + struct vdpa_device *vdpa = v->vdpa;
> >> + const struct vdpa_config_ops *ops = vdpa->config;
> >> +
> >> + if (!vdpa->use_va || !ops->bind_mm)
> >> + return 0;
> >> +
> >> + return ops->bind_mm(vdpa, v->vdev.mm);
> >> +}
> >> +
> >> +static void vhost_vdpa_unbind_mm(struct vhost_vdpa *v)
> >> +{
> >> + struct vdpa_device *vdpa = v->vdpa;
> >> + const struct vdpa_config_ops *ops = vdpa->config;
> >> +
> >> + if (!vdpa->use_va || !ops->unbind_mm)
> >> + return;
> >> +
> >> + ops->unbind_mm(vdpa);
> >> +}
> >> +
> >> static long vhost_vdpa_get_device_id(struct vhost_vdpa *v, u8 __user
> >> *argp)
> >> {
> >> struct vdpa_device *vdpa = v->vdpa;
> >> @@ -711,6 +733,13 @@ static long vhost_vdpa_unlocked_ioctl(struct file
> >> *filep,
> >> break;
> >> default:
> >> r = vhost_dev_ioctl(&v->vdev, cmd, argp);
> >> + if (!r && cmd == VHOST_SET_OWNER) {
> >> + r = vhost_vdpa_bind_mm(v);
> >> + if (r) {
> >> + vhost_dev_reset_owner(&v->vdev, NULL);
> >> + break;
> >> + }
> >> + }
> >
> >Nit: is it better to have a new condition/switch branch instead of
> >putting them under default? (as what vring_ioctl did).
>
> Yep, I agree!
>
> I'll change it.
Or maybe I can simply add `case VHOST_SET_OWNER` on this switch and call
vhost_dev_set_owner() and vhost_vdpa_bind_mm(), I mean something like
this:
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 331d4a718bf6..20250c3418b2 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -731,15 +731,16 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep,
case VHOST_VDPA_RESUME:
r = vhost_vdpa_resume(v);
break;
+ case VHOST_SET_OWNER:
+ r = vhost_dev_set_owner(d);
+ if (r)
+ break;
+ r = vhost_vdpa_bind_mm(v);
+ if (r)
+ vhost_dev_reset_owner(d, NULL);
+ break;
default:
r = vhost_dev_ioctl(&v->vdev, cmd, argp);
- if (!r && cmd == VHOST_SET_OWNER) {
- r = vhost_vdpa_bind_mm(v);
- if (r) {
- vhost_dev_reset_owner(&v->vdev, NULL);
- break;
- }
- }
if (r == -ENOIOCTLCMD)
r = vhost_vdpa_vring_ioctl(v, cmd, argp);
break;
WDYT?
Thanks,
Stefano
_______________________________________________
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
