| Message ID | 20200924032125.18619-11-jasowang@redhat.com |
|---|---|
| State | RFC |
| Delegated to: | David Miller |
| Headers | show |
| Series | Control VQ support in vDPA | expand |
On Thu, Sep 24, 2020 at 11:21:11AM +0800, Jason Wang wrote: > This patch introduces a new bus operation to allow the vDPA bus driver > to associate an ASID to a virtqueue group. > So in case of virtio_net, I would expect that all the data virtqueues will be associated with the same address space identifier. Moreover, this assignment should be provided before the set_map call that provides the iotlb for the address space, correct? > Signed-off-by: Jason Wang <jasowang@redhat.com> > --- > include/linux/vdpa.h | 10 ++++++++++ > 1 file changed, 10 insertions(+) > > diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h > index 1e1163daa352..e2394995a3cd 100644 > --- a/include/linux/vdpa.h > +++ b/include/linux/vdpa.h > @@ -160,6 +160,12 @@ struct vdpa_device { > * @get_generation: Get device config generation (optional) > * @vdev: vdpa device > * Returns u32: device generation > + * @set_group_asid: Set address space identifier for a > + * virtqueue group > + * @vdev: vdpa device > + * @group: virtqueue group > + * @asid: address space id for this group > + * Returns integer: success (0) or error (< 0) > * @set_map: Set device memory mapping (optional) > * Needed for device that using device > * specific DMA translation (on-chip IOMMU) > @@ -237,6 +243,10 @@ struct vdpa_config_ops { > u64 iova, u64 size, u64 pa, u32 perm); > int (*dma_unmap)(struct vdpa_device *vdev, unsigned int asid, > u64 iova, u64 size); > + int (*set_group_asid)(struct vdpa_device *vdev, unsigned int group, > + unsigned int asid); > + > + Extra space > > /* Free device resources */ > void (*free)(struct vdpa_device *vdev); > -- > 2.20.1 >
On 2020/10/1 下午9:29, Eli Cohen wrote: > On Thu, Sep 24, 2020 at 11:21:11AM +0800, Jason Wang wrote: >> This patch introduces a new bus operation to allow the vDPA bus driver >> to associate an ASID to a virtqueue group. >> > So in case of virtio_net, I would expect that all the data virtqueues > will be associated with the same address space identifier. Right. I will add the codes to do this in the next version. It should be more explicit than have this assumption by default. > Moreover, > this assignment should be provided before the set_map call that provides > the iotlb for the address space, correct? I think it's better not have this limitation, note that set_map() now takes a asid argument. So for hardware if the associated as is changed, the driver needs to program the hardware to switch to the new mapping. Does this work for mlx5? >> Signed-off-by: Jason Wang <jasowang@redhat.com> >> --- >> include/linux/vdpa.h | 10 ++++++++++ >> 1 file changed, 10 insertions(+) >> >> diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h >> index 1e1163daa352..e2394995a3cd 100644 >> --- a/include/linux/vdpa.h >> +++ b/include/linux/vdpa.h >> @@ -160,6 +160,12 @@ struct vdpa_device { >> * @get_generation: Get device config generation (optional) >> * @vdev: vdpa device >> * Returns u32: device generation >> + * @set_group_asid: Set address space identifier for a >> + * virtqueue group >> + * @vdev: vdpa device >> + * @group: virtqueue group >> + * @asid: address space id for this group >> + * Returns integer: success (0) or error (< 0) >> * @set_map: Set device memory mapping (optional) >> * Needed for device that using device >> * specific DMA translation (on-chip IOMMU) >> @@ -237,6 +243,10 @@ struct vdpa_config_ops { >> u64 iova, u64 size, u64 pa, u32 perm); >> int (*dma_unmap)(struct vdpa_device *vdev, unsigned int asid, >> u64 iova, u64 size); >> + int (*set_group_asid)(struct vdpa_device *vdev, unsigned int group, >> + unsigned int asid); >> + >> + > Extra space Will fix. Thanks >> >> /* Free device resources */ >> void (*free)(struct vdpa_device *vdev); >> -- >> 2.20.1 >>
On Fri, Oct 09, 2020 at 11:56:45AM +0800, Jason Wang wrote: > > On 2020/10/1 下午9:29, Eli Cohen wrote: > > On Thu, Sep 24, 2020 at 11:21:11AM +0800, Jason Wang wrote: > > > This patch introduces a new bus operation to allow the vDPA bus driver > > > to associate an ASID to a virtqueue group. > > > > > So in case of virtio_net, I would expect that all the data virtqueues > > will be associated with the same address space identifier. > > > Right. > > I will add the codes to do this in the next version. It should be more > explicit than have this assumption by default. > > > > Moreover, > > this assignment should be provided before the set_map call that provides > > the iotlb for the address space, correct? > > > I think it's better not have this limitation, note that set_map() now takes > a asid argument. > > So for hardware if the associated as is changed, the driver needs to program > the hardware to switch to the new mapping. > > Does this work for mlx5? > So in theory we can have several asid's (for different virtqueues), each one should be followed by a specific set_map call. If this is so, how do I know if I met all the conditions run my driver? Maybe we need another callback to let the driver know it should not expect more set_maps().
On 2020/10/12 下午2:59, Eli Cohen wrote: > On Fri, Oct 09, 2020 at 11:56:45AM +0800, Jason Wang wrote: >> On 2020/10/1 下午9:29, Eli Cohen wrote: >>> On Thu, Sep 24, 2020 at 11:21:11AM +0800, Jason Wang wrote: >>>> This patch introduces a new bus operation to allow the vDPA bus driver >>>> to associate an ASID to a virtqueue group. >>>> >>> So in case of virtio_net, I would expect that all the data virtqueues >>> will be associated with the same address space identifier. >> >> Right. >> >> I will add the codes to do this in the next version. It should be more >> explicit than have this assumption by default. >> >> >>> Moreover, >>> this assignment should be provided before the set_map call that provides >>> the iotlb for the address space, correct? >> >> I think it's better not have this limitation, note that set_map() now takes >> a asid argument. >> >> So for hardware if the associated as is changed, the driver needs to program >> the hardware to switch to the new mapping. >> >> Does this work for mlx5? >> > So in theory we can have several asid's (for different virtqueues), each > one should be followed by a specific set_map call. If this is so, how do > I know if I met all the conditions run my driver? Maybe we need another > callback to let the driver know it should not expect more set_maps(). This should work similarly as in the past. Two parts of the work is expected to be done by the driver: 1) store the mapping somewhere (e.g hardware) during set_map() 2) associating mapping with a specific virtqueue The only difference is that more than one mapping is used now. For the issue of more set_maps(), driver should be always ready for the new set_maps() call instead of not expecting new set_maps() since guest memory topology could be changed due to several reasons. Qemu or vhost-vDPA will try their best to avoid the frequency of set_maps() for better performance (e.g through batched IOTLB updating). E.g there should be at most one set_map() during one time of guest booting. Thanks >
On Mon, Oct 12, 2020 at 03:45:10PM +0800, Jason Wang wrote: > > > > > So in theory we can have several asid's (for different virtqueues), each > > one should be followed by a specific set_map call. If this is so, how do > > I know if I met all the conditions run my driver? Maybe we need another > > callback to let the driver know it should not expect more set_maps(). > > > This should work similarly as in the past. Two parts of the work is expected > to be done by the driver: > > 1) store the mapping somewhere (e.g hardware) during set_map() > 2) associating mapping with a specific virtqueue > > The only difference is that more than one mapping is used now. ok, so like today, I will always get DRIVER_OK after I got all the set_maps(), right? > > For the issue of more set_maps(), driver should be always ready for the new > set_maps() call instead of not expecting new set_maps() since guest memory > topology could be changed due to several reasons. > > Qemu or vhost-vDPA will try their best to avoid the frequency of set_maps() > for better performance (e.g through batched IOTLB updating). E.g there > should be at most one set_map() during one time of guest booting. > > > > >
On 2020/10/12 下午4:17, Eli Cohen wrote: > On Mon, Oct 12, 2020 at 03:45:10PM +0800, Jason Wang wrote: >>> So in theory we can have several asid's (for different virtqueues), each >>> one should be followed by a specific set_map call. If this is so, how do >>> I know if I met all the conditions run my driver? Maybe we need another >>> callback to let the driver know it should not expect more set_maps(). >> >> This should work similarly as in the past. Two parts of the work is expected >> to be done by the driver: >> >> 1) store the mapping somewhere (e.g hardware) during set_map() >> 2) associating mapping with a specific virtqueue >> >> The only difference is that more than one mapping is used now. > ok, so like today, I will always get DRIVER_OK after I got all the > set_maps(), right? Yes. Thanks > >> For the issue of more set_maps(), driver should be always ready for the new >> set_maps() call instead of not expecting new set_maps() since guest memory >> topology could be changed due to several reasons. >> >> Qemu or vhost-vDPA will try their best to avoid the frequency of set_maps() >> for better performance (e.g through batched IOTLB updating). E.g there >> should be at most one set_map() during one time of guest booting. >> >>
diff --git a/include/linux/vdpa.h b/include/linux/vdpa.h index 1e1163daa352..e2394995a3cd 100644 --- a/include/linux/vdpa.h +++ b/include/linux/vdpa.h @@ -160,6 +160,12 @@ struct vdpa_device { * @get_generation: Get device config generation (optional) * @vdev: vdpa device * Returns u32: device generation + * @set_group_asid: Set address space identifier for a + * virtqueue group + * @vdev: vdpa device + * @group: virtqueue group + * @asid: address space id for this group + * Returns integer: success (0) or error (< 0) * @set_map: Set device memory mapping (optional) * Needed for device that using device * specific DMA translation (on-chip IOMMU) @@ -237,6 +243,10 @@ struct vdpa_config_ops { u64 iova, u64 size, u64 pa, u32 perm); int (*dma_unmap)(struct vdpa_device *vdev, unsigned int asid, u64 iova, u64 size); + int (*set_group_asid)(struct vdpa_device *vdev, unsigned int group, + unsigned int asid); + + /* Free device resources */ void (*free)(struct vdpa_device *vdev);
This patch introduces a new bus operation to allow the vDPA bus driver to associate an ASID to a virtqueue group. Signed-off-by: Jason Wang <jasowang@redhat.com> --- include/linux/vdpa.h | 10 ++++++++++ 1 file changed, 10 insertions(+)