> From: Jason Wang <jasowang@xxxxxxxxxx>
> Sent: Wednesday, April 7, 2021 8:48 AM
[..]
> > +/**
> > + * vdpa_set_features - Set vDPA device features
> > + * @vdev: vdpa device whose features to set
> > + * @features: features of the vdpa device to enable/disable
> > + *
> > + * Return: Returns 0 on success or error code.
> > + */
> > +int vdpa_set_features(struct vdpa_device *vdev, u64 features) {
> > + const struct vdpa_config_ops *ops = vdev->config;
> > +
> > + vdev->features_valid = true;
> > + return ops->set_features(vdev, features); }
> > +EXPORT_SYMBOL_GPL(vdpa_set_features);
> > +
>
>
> Let's add a doc for this function?
Kdoc comment is added above the function in this patch.
> > --- a/drivers/vhost/vdpa.c
> > +++ b/drivers/vhost/vdpa.c
>
>
> We probably need to convert drivers/virtio/vdpa.c as well.
Yes, will do.
>
>
> > @@ -236,7 +236,6 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> > struct vhost_vdpa_config __user *c)
> > {
> > struct vdpa_device *vdpa = v->vdpa;
> > - const struct vdpa_config_ops *ops = vdpa->config;
> > struct vhost_vdpa_config config;
> > unsigned long size = offsetof(struct vhost_vdpa_config, buf);
> > u8 *buf;
> > @@ -250,7 +249,7 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> > if (IS_ERR(buf))
> > return PTR_ERR(buf);
> >
> > - ops->set_config(vdpa, config.off, buf, config.len);
> > + vdpa_set_config(vdpa, config.off, buf, config.len);
> >
> > kvfree(buf);
> > return 0;
> > @@ -259,10 +258,9 @@ static long vhost_vdpa_set_config(struct
> vhost_vdpa *v,
> > static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user
> *featurep)
> > {
> > struct vdpa_device *vdpa = v->vdpa;
> > - const struct vdpa_config_ops *ops = vdpa->config;
> > u64 features;
> >
> > - features = ops->get_features(vdpa);
> > + features = vdpa_get_features(vdpa);
> >
> > if (copy_to_user(featurep, &features, sizeof(features)))
> > return -EFAULT;
> > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index
> > 37b65ca940cf..62e68ccc4c96 100644
> > --- a/include/linux/vdpa.h
> > +++ b/include/linux/vdpa.h
> > @@ -320,28 +320,12 @@ static inline void vdpa_reset(struct vdpa_device
> *vdev)
> > ops->set_status(vdev, 0);
> > }
> >
> > -static inline int vdpa_set_features(struct vdpa_device *vdev, u64
> > features) -{
> > - const struct vdpa_config_ops *ops = vdev->config;
> > -
> > - vdev->features_valid = true;
> > - return ops->set_features(vdev, features);
> > -}
> > -
> > -
> > -static inline void vdpa_get_config(struct vdpa_device *vdev, unsigned
> offset,
> > - void *buf, unsigned int len)
> > -{
> > - const struct vdpa_config_ops *ops = vdev->config;
> > -
> > - /*
> > - * Config accesses aren't supposed to trigger before features are set.
> > - * If it does happen we assume a legacy guest.
> > - */
> > - if (!vdev->features_valid)
> > - vdpa_set_features(vdev, 0);
> > - ops->get_config(vdev, offset, buf, len);
> > -}
> > +u64 vdpa_get_features(struct vdpa_device *vdev); int
> > +vdpa_set_features(struct vdpa_device *vdev, u64 features); void
> > +vdpa_get_config(struct vdpa_device *vdev, unsigned int offset,
> > + void *buf, unsigned int len);
> > +void vdpa_set_config(struct vdpa_device *dev, unsigned int offset,
> > + void *buf, unsigned int length);
>
>
> I think it's better to use a separate patch for this moving.
>
Ok. will have prepatch to this one for movement.
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
