Jason Wang
2023-Aug-10 03:49 UTC
[PATCH RFC 2/3] vhost-vdpa: introduce descriptor group backend feature
On Wed, Aug 9, 2023 at 8:56?PM Si-Wei Liu <si-wei.liu at oracle.com> wrote:> > Userspace knows if the device has dedicated descriptor group or not > by checking this feature bit. > > It's only exposed if the vdpa driver backend implements the > .get_vq_desc_group() operation callback. Userspace trying to negotiate > this feature when it or the dependent _F_IOTLB_ASID feature hasn't > been exposed will result in an error. > > Signed-off-by: Si-Wei Liu <si-wei.liu at oracle.com> > --- > drivers/vhost/vdpa.c | 17 +++++++++++++++++ > include/uapi/linux/vhost_types.h | 5 +++++ > 2 files changed, 22 insertions(+) > > diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c > index b43e868..f2e5dce 100644 > --- a/drivers/vhost/vdpa.c > +++ b/drivers/vhost/vdpa.c > @@ -389,6 +389,14 @@ static bool vhost_vdpa_can_resume(const struct vhost_vdpa *v) > return ops->resume; > } > > +static bool vhost_vdpa_has_desc_group(const struct vhost_vdpa *v) > +{ > + struct vdpa_device *vdpa = v->vdpa; > + const struct vdpa_config_ops *ops = vdpa->config; > + > + return ops->get_vq_desc_group; > +} > + > static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep) > { > struct vdpa_device *vdpa = v->vdpa; > @@ -679,6 +687,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > if (copy_from_user(&features, featurep, sizeof(features))) > return -EFAULT; > if (features & ~(VHOST_VDPA_BACKEND_FEATURES | > + BIT_ULL(VHOST_BACKEND_F_DESC_ASID) | > BIT_ULL(VHOST_BACKEND_F_SUSPEND) | > BIT_ULL(VHOST_BACKEND_F_RESUME))) > return -EOPNOTSUPP; > @@ -688,6 +697,12 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > if ((features & BIT_ULL(VHOST_BACKEND_F_RESUME)) && > !vhost_vdpa_can_resume(v)) > return -EOPNOTSUPP; > + if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) && > + !(features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) > + return -EINVAL; > + if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) && > + !vhost_vdpa_has_desc_group(v)) > + return -EOPNOTSUPP; > vhost_set_backend_features(&v->vdev, features); > return 0; > } > @@ -741,6 +756,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, > features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND); > if (vhost_vdpa_can_resume(v)) > features |= BIT_ULL(VHOST_BACKEND_F_RESUME); > + if (vhost_vdpa_has_desc_group(v)) > + features |= BIT_ULL(VHOST_BACKEND_F_DESC_ASID); > if (copy_to_user(featurep, &features, sizeof(features))) > r = -EFAULT; > break; > diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h > index d3aad12a..0856f84 100644 > --- a/include/uapi/linux/vhost_types.h > +++ b/include/uapi/linux/vhost_types.h > @@ -181,5 +181,10 @@ struct vhost_vdpa_iova_range { > #define VHOST_BACKEND_F_SUSPEND 0x4 > /* Device can be resumed */ > #define VHOST_BACKEND_F_RESUME 0x5 > +/* Device may expose the descriptor table, avail and used ring in a > + * different group for ASID binding than the buffers it contains.Nit: s/a different group/different groups/? Btw, not a native speaker but I think "descriptor" might be confusing since as you explained above, it contains more than just a descriptor table. Thanks> + * Requires VHOST_BACKEND_F_IOTLB_ASID. > + */ > +#define VHOST_BACKEND_F_DESC_ASID 0x6 > > #endif > -- > 1.8.3.1 >
Si-Wei Liu
2023-Aug-15 23:54 UTC
[PATCH RFC 2/3] vhost-vdpa: introduce descriptor group backend feature
On 8/9/2023 8:49 PM, Jason Wang wrote:> On Wed, Aug 9, 2023 at 8:56?PM Si-Wei Liu <si-wei.liu at oracle.com> wrote: >> Userspace knows if the device has dedicated descriptor group or not >> by checking this feature bit. >> >> It's only exposed if the vdpa driver backend implements the >> .get_vq_desc_group() operation callback. Userspace trying to negotiate >> this feature when it or the dependent _F_IOTLB_ASID feature hasn't >> been exposed will result in an error. >> >> Signed-off-by: Si-Wei Liu <si-wei.liu at oracle.com> >> --- >> drivers/vhost/vdpa.c | 17 +++++++++++++++++ >> include/uapi/linux/vhost_types.h | 5 +++++ >> 2 files changed, 22 insertions(+) >> >> diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c >> index b43e868..f2e5dce 100644 >> --- a/drivers/vhost/vdpa.c >> +++ b/drivers/vhost/vdpa.c >> @@ -389,6 +389,14 @@ static bool vhost_vdpa_can_resume(const struct vhost_vdpa *v) >> return ops->resume; >> } >> >> +static bool vhost_vdpa_has_desc_group(const struct vhost_vdpa *v) >> +{ >> + struct vdpa_device *vdpa = v->vdpa; >> + const struct vdpa_config_ops *ops = vdpa->config; >> + >> + return ops->get_vq_desc_group; >> +} >> + >> static long vhost_vdpa_get_features(struct vhost_vdpa *v, u64 __user *featurep) >> { >> struct vdpa_device *vdpa = v->vdpa; >> @@ -679,6 +687,7 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, >> if (copy_from_user(&features, featurep, sizeof(features))) >> return -EFAULT; >> if (features & ~(VHOST_VDPA_BACKEND_FEATURES | >> + BIT_ULL(VHOST_BACKEND_F_DESC_ASID) | >> BIT_ULL(VHOST_BACKEND_F_SUSPEND) | >> BIT_ULL(VHOST_BACKEND_F_RESUME))) >> return -EOPNOTSUPP; >> @@ -688,6 +697,12 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, >> if ((features & BIT_ULL(VHOST_BACKEND_F_RESUME)) && >> !vhost_vdpa_can_resume(v)) >> return -EOPNOTSUPP; >> + if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) && >> + !(features & BIT_ULL(VHOST_BACKEND_F_IOTLB_ASID))) >> + return -EINVAL; >> + if ((features & BIT_ULL(VHOST_BACKEND_F_DESC_ASID)) && >> + !vhost_vdpa_has_desc_group(v)) >> + return -EOPNOTSUPP; >> vhost_set_backend_features(&v->vdev, features); >> return 0; >> } >> @@ -741,6 +756,8 @@ static long vhost_vdpa_unlocked_ioctl(struct file *filep, >> features |= BIT_ULL(VHOST_BACKEND_F_SUSPEND); >> if (vhost_vdpa_can_resume(v)) >> features |= BIT_ULL(VHOST_BACKEND_F_RESUME); >> + if (vhost_vdpa_has_desc_group(v)) >> + features |= BIT_ULL(VHOST_BACKEND_F_DESC_ASID); >> if (copy_to_user(featurep, &features, sizeof(features))) >> r = -EFAULT; >> break; >> diff --git a/include/uapi/linux/vhost_types.h b/include/uapi/linux/vhost_types.h >> index d3aad12a..0856f84 100644 >> --- a/include/uapi/linux/vhost_types.h >> +++ b/include/uapi/linux/vhost_types.h >> @@ -181,5 +181,10 @@ struct vhost_vdpa_iova_range { >> #define VHOST_BACKEND_F_SUSPEND 0x4 >> /* Device can be resumed */ >> #define VHOST_BACKEND_F_RESUME 0x5 >> +/* Device may expose the descriptor table, avail and used ring in a >> + * different group for ASID binding than the buffers it contains. > Nit: > > s/a different group/different groups/?Yep, I will try to rephrase. Would below work? "Device may expose virtqueue's descriptor table, avail and used ring in a different group for ASID binding than where buffers it contains reside."> Btw, not a native speaker but I think "descriptor" might be confusing > since as you explained above, it contains more than just a descriptor > table.Yep. I chose "descriptor" because packed virtqueue doesn't have "physical" avail and used ring other than descriptor table, but I think I am open to a better name. I once thought of "descriptor ring" but that might be too specific to packed virtqueue. Any suggestion? Thanks, -Siwei> > Thanks > >> + * Requires VHOST_BACKEND_F_IOTLB_ASID. >> + */ >> +#define VHOST_BACKEND_F_DESC_ASID 0x6 >> >> #endif >> -- >> 1.8.3.1 >>