diff mbox series

[RFC,10/24] vdpa: introduce config operations for associating ASID to a virtqueue group

Message ID 20200924032125.18619-11-jasowang@redhat.com
State New
Headers show
Series Control VQ support in vDPA | expand

Commit Message

Jason Wang Sept. 24, 2020, 3:21 a.m. UTC
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(+)

Comments

Eli Cohen Oct. 1, 2020, 1:29 p.m. UTC | #1
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
>
Jason Wang Oct. 9, 2020, 3:56 a.m. UTC | #2
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
>>
Eli Cohen Oct. 12, 2020, 6:59 a.m. UTC | #3
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().
Jason Wang Oct. 12, 2020, 7:45 a.m. UTC | #4
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


>
Eli Cohen Oct. 12, 2020, 8:17 a.m. UTC | #5
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.
> 
> 
> > 
>
Jason Wang Oct. 13, 2020, 5:40 a.m. UTC | #6
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 mbox series

Patch

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);