Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-02 Thread Xuan Zhuo
On Thu, 2 Mar 2023 15:56:42 +0800, Jason Wang  wrote:
> On Thu, Mar 2, 2023 at 3:34 PM Xuan Zhuo  wrote:
> >
> > On Thu, 2 Mar 2023 14:56:11 +0800, Jason Wang  wrote:
> > > On Thu, Mar 2, 2023 at 2:09 PM Michael S. Tsirkin  wrote:
> > > >
> > > > On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> > > > > On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo 
> > > > >  wrote:
> > > > > >
> > > > > > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  
> > > > > > wrote:
> > > > > > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > > > > > >  wrote:
> > > > > > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang 
> > > > > > > > >  wrote:
> > > > > > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > > > > > >  wrote:
> > > > > > > > > > > > >
> > > > > > > > > > > > > Added virtio_dma_map() to map DMA addresses for 
> > > > > > > > > > > > > virtual memory in
> > > > > > > > > > > > > advance. The purpose is to keep memory mapped across 
> > > > > > > > > > > > > multiple add/get
> > > > > > > > > > > > > buf operations.
> > > > > > > > > > > >
> > > > > > > > > > > > I wonder if instead of exporting helpers like this, it 
> > > > > > > > > > > > might be simple
> > > > > > > > > > > > to just export dma_dev then the upper layer can use DMA 
> > > > > > > > > > > > API at will?
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > The reason for not doing this, Virtio is not just using 
> > > > > > > > > > > DMA_DEV to mapp, but
> > > > > > > > > > > also check whether DMA is used.
> > > > > > > > > >
> > > > > > > > > > We should let the DMA API decide by exporting a correct 
> > > > > > > > > > dma_dev. E.g
> > > > > > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA 
> > > > > > > > > > dev without
> > > > > > > > > > dma_ops.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > Do you mean we provide this API?
> > > > > > > > >
> > > > > > > > > virtio_get_dma_dev()
> > > > > > > > >
> > > > > > > > > If it returns NULL, the caller will use the physical memory 
> > > > > > > > > address directly. If
> > > > > > > > > this func return a dma_dev, the caller should use DMA API.
> > > > > > > >
> > > > > > > >
> > > > > > > > cc the XDP_SOCKET's maintainers.
> > > > > > > >
> > > > > > > > First of all, Jason does not want to encapsulate the API of DMA 
> > > > > > > > by Virtio. It is
> > > > > > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP 
> > > > > > > > operation directly.
> > > > > > > > I agree with this idea.
> > > > > > > >
> > > > > > > > However, there are several problems under Virtio:
> > > > > > > > 1. In some virtualization scenarios, we do not have to perform 
> > > > > > > > DMA operations,
> > > > > > > >just use the physical address directly.
> > > > > > >
> > > > > > > This is not a problem, we can simply return the virtio device 
> > > > > > > itself
> > > > > > > as the DMA device in this case. Since there's no DMA ops 
> > > > > > > attached, DMA
> > > > > > > API will use physical address in this case.
> > > > > >
> > > > > > Is this like this? So why do we have to deal with it in Virtio 
> > > > > > Ring? Let me
> > > > > > learn it.
> > > > >
> > > > > It has a long debate and I can't recall too many details. (You can
> > > > > search the archives). Michael may show more thoughts here.
> > > > >
> > > > > One concern is the overhead of the DMA API that needs to be 
> > > > > benchmarked.
> > > > >
> > > > > Thanks
> > > >
> > > > Concern with what?
> > >
> > > Always use the DMA API for virtio devices by drop vq->use_dma_api.
> >
> > Do you mean the affects of the AF_XDP performance?
>
> No, I was replied to your question:
>
> "
> Is this like this? So why do we have to deal with it in Virtio Ring?
> "
>
> Try to answer why we don't do that in the virtio ring level but try to
> have virtio's own dma helper like vring_map_one_sg().

I see.

Thanks.


>
> > I think this
> > may not be a key issue, because DMA is completed in advance, and there is no
> > DMA operation on the data path. So the overhead is not big.
>
> Right.
>
> Thanks
>
> >
> > Thanks.
> >
> >
> > >
> > > For the device that doesn't need quirk, advertise the virtio device as
> > > its dma device.
> > >
> > > Thanks
> > >
> > > > This patch does not change devices, they are using
> > > > the existing API. Xuan Zhuo already showed a benchmark result for 
> > > > AF_XDP.
> > > >
> > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > > 2. The latest Virtio Core supports each rx/tx queue with one 
> > > > > > > > DMA device.
> > > > > > > >Generally, the physical network card has only one device. 
> > > > > > > > All queues use
> > > > > 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Jason Wang
On Thu, Mar 2, 2023 at 3:34 PM Xuan Zhuo  wrote:
>
> On Thu, 2 Mar 2023 14:56:11 +0800, Jason Wang  wrote:
> > On Thu, Mar 2, 2023 at 2:09 PM Michael S. Tsirkin  wrote:
> > >
> > > On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> > > > On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  
> > > > wrote:
> > > > >
> > > > > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  
> > > > > wrote:
> > > > > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > > > > >  wrote:
> > > > > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang 
> > > > > > > >  wrote:
> > > > > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > > > > >  wrote:
> > > > > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > > > > >  wrote:
> > > > > > > > > > > >
> > > > > > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual 
> > > > > > > > > > > > memory in
> > > > > > > > > > > > advance. The purpose is to keep memory mapped across 
> > > > > > > > > > > > multiple add/get
> > > > > > > > > > > > buf operations.
> > > > > > > > > > >
> > > > > > > > > > > I wonder if instead of exporting helpers like this, it 
> > > > > > > > > > > might be simple
> > > > > > > > > > > to just export dma_dev then the upper layer can use DMA 
> > > > > > > > > > > API at will?
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > The reason for not doing this, Virtio is not just using 
> > > > > > > > > > DMA_DEV to mapp, but
> > > > > > > > > > also check whether DMA is used.
> > > > > > > > >
> > > > > > > > > We should let the DMA API decide by exporting a correct 
> > > > > > > > > dma_dev. E.g
> > > > > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev 
> > > > > > > > > without
> > > > > > > > > dma_ops.
> > > > > > > >
> > > > > > > >
> > > > > > > > Do you mean we provide this API?
> > > > > > > >
> > > > > > > > virtio_get_dma_dev()
> > > > > > > >
> > > > > > > > If it returns NULL, the caller will use the physical memory 
> > > > > > > > address directly. If
> > > > > > > > this func return a dma_dev, the caller should use DMA API.
> > > > > > >
> > > > > > >
> > > > > > > cc the XDP_SOCKET's maintainers.
> > > > > > >
> > > > > > > First of all, Jason does not want to encapsulate the API of DMA 
> > > > > > > by Virtio. It is
> > > > > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP 
> > > > > > > operation directly.
> > > > > > > I agree with this idea.
> > > > > > >
> > > > > > > However, there are several problems under Virtio:
> > > > > > > 1. In some virtualization scenarios, we do not have to perform 
> > > > > > > DMA operations,
> > > > > > >just use the physical address directly.
> > > > > >
> > > > > > This is not a problem, we can simply return the virtio device itself
> > > > > > as the DMA device in this case. Since there's no DMA ops attached, 
> > > > > > DMA
> > > > > > API will use physical address in this case.
> > > > >
> > > > > Is this like this? So why do we have to deal with it in Virtio Ring? 
> > > > > Let me
> > > > > learn it.
> > > >
> > > > It has a long debate and I can't recall too many details. (You can
> > > > search the archives). Michael may show more thoughts here.
> > > >
> > > > One concern is the overhead of the DMA API that needs to be benchmarked.
> > > >
> > > > Thanks
> > >
> > > Concern with what?
> >
> > Always use the DMA API for virtio devices by drop vq->use_dma_api.
>
> Do you mean the affects of the AF_XDP performance?

No, I was replied to your question:

"
Is this like this? So why do we have to deal with it in Virtio Ring?
"

Try to answer why we don't do that in the virtio ring level but try to
have virtio's own dma helper like vring_map_one_sg().

> I think this
> may not be a key issue, because DMA is completed in advance, and there is no
> DMA operation on the data path. So the overhead is not big.

Right.

Thanks

>
> Thanks.
>
>
> >
> > For the device that doesn't need quirk, advertise the virtio device as
> > its dma device.
> >
> > Thanks
> >
> > > This patch does not change devices, they are using
> > > the existing API. Xuan Zhuo already showed a benchmark result for AF_XDP.
> > >
> > >
> > > > >
> > > > >
> > > > > >
> > > > > > > 2. The latest Virtio Core supports each rx/tx queue with one DMA 
> > > > > > > device.
> > > > > > >Generally, the physical network card has only one device. All 
> > > > > > > queues use
> > > > > > >it for DMA operation.
> > > > > >
> > > > > > I'm not sure this is a big deal, we just need to use the per 
> > > > > > virtqueue
> > > > > > dma device to use DMA API.
> > > > >
> > > > > Yes.
> > > > >
> > > > >
> > > > > >
> > > > > > >
> > > > > > > So I consider this problem again, Virtio Core provides only one 
> > > > > > > API.
> > > > > > >
> > 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Xuan Zhuo
On Thu, 2 Mar 2023 14:56:11 +0800, Jason Wang  wrote:
> On Thu, Mar 2, 2023 at 2:09 PM Michael S. Tsirkin  wrote:
> >
> > On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> > > On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  
> > > wrote:
> > > >
> > > > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  
> > > > wrote:
> > > > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo 
> > > > >  wrote:
> > > > > >
> > > > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > > > >  wrote:
> > > > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang 
> > > > > > >  wrote:
> > > > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > > > >  wrote:
> > > > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > > > >  wrote:
> > > > > > > > > > >
> > > > > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual 
> > > > > > > > > > > memory in
> > > > > > > > > > > advance. The purpose is to keep memory mapped across 
> > > > > > > > > > > multiple add/get
> > > > > > > > > > > buf operations.
> > > > > > > > > >
> > > > > > > > > > I wonder if instead of exporting helpers like this, it 
> > > > > > > > > > might be simple
> > > > > > > > > > to just export dma_dev then the upper layer can use DMA API 
> > > > > > > > > > at will?
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > The reason for not doing this, Virtio is not just using 
> > > > > > > > > DMA_DEV to mapp, but
> > > > > > > > > also check whether DMA is used.
> > > > > > > >
> > > > > > > > We should let the DMA API decide by exporting a correct 
> > > > > > > > dma_dev. E.g
> > > > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev 
> > > > > > > > without
> > > > > > > > dma_ops.
> > > > > > >
> > > > > > >
> > > > > > > Do you mean we provide this API?
> > > > > > >
> > > > > > > virtio_get_dma_dev()
> > > > > > >
> > > > > > > If it returns NULL, the caller will use the physical memory 
> > > > > > > address directly. If
> > > > > > > this func return a dma_dev, the caller should use DMA API.
> > > > > >
> > > > > >
> > > > > > cc the XDP_SOCKET's maintainers.
> > > > > >
> > > > > > First of all, Jason does not want to encapsulate the API of DMA by 
> > > > > > Virtio. It is
> > > > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP 
> > > > > > operation directly.
> > > > > > I agree with this idea.
> > > > > >
> > > > > > However, there are several problems under Virtio:
> > > > > > 1. In some virtualization scenarios, we do not have to perform DMA 
> > > > > > operations,
> > > > > >just use the physical address directly.
> > > > >
> > > > > This is not a problem, we can simply return the virtio device itself
> > > > > as the DMA device in this case. Since there's no DMA ops attached, DMA
> > > > > API will use physical address in this case.
> > > >
> > > > Is this like this? So why do we have to deal with it in Virtio Ring? 
> > > > Let me
> > > > learn it.
> > >
> > > It has a long debate and I can't recall too many details. (You can
> > > search the archives). Michael may show more thoughts here.
> > >
> > > One concern is the overhead of the DMA API that needs to be benchmarked.
> > >
> > > Thanks
> >
> > Concern with what?
>
> Always use the DMA API for virtio devices by drop vq->use_dma_api.

Do you mean the affects of the AF_XDP performance? I think this
may not be a key issue, because DMA is completed in advance, and there is no
DMA operation on the data path. So the overhead is not big.

Thanks.


>
> For the device that doesn't need quirk, advertise the virtio device as
> its dma device.
>
> Thanks
>
> > This patch does not change devices, they are using
> > the existing API. Xuan Zhuo already showed a benchmark result for AF_XDP.
> >
> >
> > > >
> > > >
> > > > >
> > > > > > 2. The latest Virtio Core supports each rx/tx queue with one DMA 
> > > > > > device.
> > > > > >Generally, the physical network card has only one device. All 
> > > > > > queues use
> > > > > >it for DMA operation.
> > > > >
> > > > > I'm not sure this is a big deal, we just need to use the per virtqueue
> > > > > dma device to use DMA API.
> > > >
> > > > Yes.
> > > >
> > > >
> > > > >
> > > > > >
> > > > > > So I consider this problem again, Virtio Core provides only one API.
> > > > > >
> > > > > > * virtio_get_dma_dev(queue)
> > > > > >
> > > > > > If the return value is NULL, it means that there is no DMA 
> > > > > > operation. If it is
> > > > > > not NULL, use DMA API for DMA operation.
> > > > > >
> > > > > > The modification of XSK is like this. We may pass NULL as dev to 
> > > > > > xp_dma_map().
> > > > > > If dev is NULL, then there is no need to perform DMA and Sync 
> > > > > > operations.
> > > > > > Otherwise, it will perform DMA operations like other devices.
> > > > >
> > > > > As discussed above, it might be easier:
> > > > >
> > > > > if 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Jason Wang
On Thu, Mar 2, 2023 at 2:09 PM Michael S. Tsirkin  wrote:
>
> On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> > On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  
> > wrote:
> > >
> > > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  wrote:
> > > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  
> > > > wrote:
> > > > >
> > > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > > >  wrote:
> > > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang 
> > > > > >  wrote:
> > > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > > >  wrote:
> > > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual 
> > > > > > > > > > memory in
> > > > > > > > > > advance. The purpose is to keep memory mapped across 
> > > > > > > > > > multiple add/get
> > > > > > > > > > buf operations.
> > > > > > > > >
> > > > > > > > > I wonder if instead of exporting helpers like this, it might 
> > > > > > > > > be simple
> > > > > > > > > to just export dma_dev then the upper layer can use DMA API 
> > > > > > > > > at will?
> > > > > > > >
> > > > > > > >
> > > > > > > > The reason for not doing this, Virtio is not just using DMA_DEV 
> > > > > > > > to mapp, but
> > > > > > > > also check whether DMA is used.
> > > > > > >
> > > > > > > We should let the DMA API decide by exporting a correct dma_dev. 
> > > > > > > E.g
> > > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev 
> > > > > > > without
> > > > > > > dma_ops.
> > > > > >
> > > > > >
> > > > > > Do you mean we provide this API?
> > > > > >
> > > > > > virtio_get_dma_dev()
> > > > > >
> > > > > > If it returns NULL, the caller will use the physical memory address 
> > > > > > directly. If
> > > > > > this func return a dma_dev, the caller should use DMA API.
> > > > >
> > > > >
> > > > > cc the XDP_SOCKET's maintainers.
> > > > >
> > > > > First of all, Jason does not want to encapsulate the API of DMA by 
> > > > > Virtio. It is
> > > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP 
> > > > > operation directly.
> > > > > I agree with this idea.
> > > > >
> > > > > However, there are several problems under Virtio:
> > > > > 1. In some virtualization scenarios, we do not have to perform DMA 
> > > > > operations,
> > > > >just use the physical address directly.
> > > >
> > > > This is not a problem, we can simply return the virtio device itself
> > > > as the DMA device in this case. Since there's no DMA ops attached, DMA
> > > > API will use physical address in this case.
> > >
> > > Is this like this? So why do we have to deal with it in Virtio Ring? Let 
> > > me
> > > learn it.
> >
> > It has a long debate and I can't recall too many details. (You can
> > search the archives). Michael may show more thoughts here.
> >
> > One concern is the overhead of the DMA API that needs to be benchmarked.
> >
> > Thanks
>
> Concern with what?

Always use the DMA API for virtio devices by drop vq->use_dma_api.

For the device that doesn't need quirk, advertise the virtio device as
its dma device.

Thanks

> This patch does not change devices, they are using
> the existing API. Xuan Zhuo already showed a benchmark result for AF_XDP.
>
>
> > >
> > >
> > > >
> > > > > 2. The latest Virtio Core supports each rx/tx queue with one DMA 
> > > > > device.
> > > > >Generally, the physical network card has only one device. All 
> > > > > queues use
> > > > >it for DMA operation.
> > > >
> > > > I'm not sure this is a big deal, we just need to use the per virtqueue
> > > > dma device to use DMA API.
> > >
> > > Yes.
> > >
> > >
> > > >
> > > > >
> > > > > So I consider this problem again, Virtio Core provides only one API.
> > > > >
> > > > > * virtio_get_dma_dev(queue)
> > > > >
> > > > > If the return value is NULL, it means that there is no DMA operation. 
> > > > > If it is
> > > > > not NULL, use DMA API for DMA operation.
> > > > >
> > > > > The modification of XSK is like this. We may pass NULL as dev to 
> > > > > xp_dma_map().
> > > > > If dev is NULL, then there is no need to perform DMA and Sync 
> > > > > operations.
> > > > > Otherwise, it will perform DMA operations like other devices.
> > > >
> > > > As discussed above, it might be easier:
> > > >
> > > > if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
> > > > return virtio_device;
> > > > else
> > > > return vring_dma_dev();
> > >
> > > Yes, according to Jason's opinion, then XSK not need to do any 
> > > modifications.
> > >
> > > Thanks.
>
> Yes AF_XDP does not need the per VQ device hack.
> We should probably rethink it.
>
> But as far as implementation goes, poking at VIRTIO_F_ACCESS_PLATFORM
> is wrong. Please use virtio_has_dma_quirk.
>
>
>
> > > >
> > > > >
> > > > > And if the dma_dev of rx and tx is 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Xuan Zhuo
On Thu, 2 Mar 2023 01:09:04 -0500, "Michael S. Tsirkin"  wrote:
> On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> > On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  
> > wrote:
> > >
> > > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  wrote:
> > > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  
> > > > wrote:
> > > > >
> > > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > > >  wrote:
> > > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang 
> > > > > >  wrote:
> > > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > > >  wrote:
> > > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > > >  wrote:
> > > > > > > > > >
> > > > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual 
> > > > > > > > > > memory in
> > > > > > > > > > advance. The purpose is to keep memory mapped across 
> > > > > > > > > > multiple add/get
> > > > > > > > > > buf operations.
> > > > > > > > >
> > > > > > > > > I wonder if instead of exporting helpers like this, it might 
> > > > > > > > > be simple
> > > > > > > > > to just export dma_dev then the upper layer can use DMA API 
> > > > > > > > > at will?
> > > > > > > >
> > > > > > > >
> > > > > > > > The reason for not doing this, Virtio is not just using DMA_DEV 
> > > > > > > > to mapp, but
> > > > > > > > also check whether DMA is used.
> > > > > > >
> > > > > > > We should let the DMA API decide by exporting a correct dma_dev. 
> > > > > > > E.g
> > > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev 
> > > > > > > without
> > > > > > > dma_ops.
> > > > > >
> > > > > >
> > > > > > Do you mean we provide this API?
> > > > > >
> > > > > > virtio_get_dma_dev()
> > > > > >
> > > > > > If it returns NULL, the caller will use the physical memory address 
> > > > > > directly. If
> > > > > > this func return a dma_dev, the caller should use DMA API.
> > > > >
> > > > >
> > > > > cc the XDP_SOCKET's maintainers.
> > > > >
> > > > > First of all, Jason does not want to encapsulate the API of DMA by 
> > > > > Virtio. It is
> > > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP 
> > > > > operation directly.
> > > > > I agree with this idea.
> > > > >
> > > > > However, there are several problems under Virtio:
> > > > > 1. In some virtualization scenarios, we do not have to perform DMA 
> > > > > operations,
> > > > >just use the physical address directly.
> > > >
> > > > This is not a problem, we can simply return the virtio device itself
> > > > as the DMA device in this case. Since there's no DMA ops attached, DMA
> > > > API will use physical address in this case.
> > >
> > > Is this like this? So why do we have to deal with it in Virtio Ring? Let 
> > > me
> > > learn it.
> >
> > It has a long debate and I can't recall too many details. (You can
> > search the archives). Michael may show more thoughts here.
> >
> > One concern is the overhead of the DMA API that needs to be benchmarked.
> >
> > Thanks
>
> Concern with what? This patch does not change devices, they are using
> the existing API. Xuan Zhuo already showed a benchmark result for AF_XDP.
>
>
> > >
> > >
> > > >
> > > > > 2. The latest Virtio Core supports each rx/tx queue with one DMA 
> > > > > device.
> > > > >Generally, the physical network card has only one device. All 
> > > > > queues use
> > > > >it for DMA operation.
> > > >
> > > > I'm not sure this is a big deal, we just need to use the per virtqueue
> > > > dma device to use DMA API.
> > >
> > > Yes.
> > >
> > >
> > > >
> > > > >
> > > > > So I consider this problem again, Virtio Core provides only one API.
> > > > >
> > > > > * virtio_get_dma_dev(queue)
> > > > >
> > > > > If the return value is NULL, it means that there is no DMA operation. 
> > > > > If it is
> > > > > not NULL, use DMA API for DMA operation.
> > > > >
> > > > > The modification of XSK is like this. We may pass NULL as dev to 
> > > > > xp_dma_map().
> > > > > If dev is NULL, then there is no need to perform DMA and Sync 
> > > > > operations.
> > > > > Otherwise, it will perform DMA operations like other devices.
> > > >
> > > > As discussed above, it might be easier:
> > > >
> > > > if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
> > > > return virtio_device;
> > > > else
> > > > return vring_dma_dev();
> > >
> > > Yes, according to Jason's opinion, then XSK not need to do any 
> > > modifications.
> > >
> > > Thanks.
>
> Yes AF_XDP does not need the per VQ device hack.
> We should probably rethink it.
>
> But as far as implementation goes, poking at VIRTIO_F_ACCESS_PLATFORM
> is wrong. Please use virtio_has_dma_quirk.

I think the code should like this:

+struct device *virtqueue_get_dma_dev(struct virtqueue *_vq)
+{
+   struct vring_virtqueue *vq = to_vvq(_vq);
+
+   if (!vq->use_dma_api)
+   return >vq.vdev->dev;
+
+   return 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Michael S. Tsirkin
On Thu, Mar 02, 2023 at 11:26:53AM +0800, Jason Wang wrote:
> On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  wrote:
> >
> > On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  wrote:
> > > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  
> > > wrote:
> > > >
> > > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > > >  wrote:
> > > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  
> > > > > wrote:
> > > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > > >  wrote:
> > > > > > >
> > > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > > >  wrote:
> > > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > > >  wrote:
> > > > > > > > >
> > > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual 
> > > > > > > > > memory in
> > > > > > > > > advance. The purpose is to keep memory mapped across multiple 
> > > > > > > > > add/get
> > > > > > > > > buf operations.
> > > > > > > >
> > > > > > > > I wonder if instead of exporting helpers like this, it might be 
> > > > > > > > simple
> > > > > > > > to just export dma_dev then the upper layer can use DMA API at 
> > > > > > > > will?
> > > > > > >
> > > > > > >
> > > > > > > The reason for not doing this, Virtio is not just using DMA_DEV 
> > > > > > > to mapp, but
> > > > > > > also check whether DMA is used.
> > > > > >
> > > > > > We should let the DMA API decide by exporting a correct dma_dev. E.g
> > > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev 
> > > > > > without
> > > > > > dma_ops.
> > > > >
> > > > >
> > > > > Do you mean we provide this API?
> > > > >
> > > > > virtio_get_dma_dev()
> > > > >
> > > > > If it returns NULL, the caller will use the physical memory address 
> > > > > directly. If
> > > > > this func return a dma_dev, the caller should use DMA API.
> > > >
> > > >
> > > > cc the XDP_SOCKET's maintainers.
> > > >
> > > > First of all, Jason does not want to encapsulate the API of DMA by 
> > > > Virtio. It is
> > > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP operation 
> > > > directly.
> > > > I agree with this idea.
> > > >
> > > > However, there are several problems under Virtio:
> > > > 1. In some virtualization scenarios, we do not have to perform DMA 
> > > > operations,
> > > >just use the physical address directly.
> > >
> > > This is not a problem, we can simply return the virtio device itself
> > > as the DMA device in this case. Since there's no DMA ops attached, DMA
> > > API will use physical address in this case.
> >
> > Is this like this? So why do we have to deal with it in Virtio Ring? Let me
> > learn it.
> 
> It has a long debate and I can't recall too many details. (You can
> search the archives). Michael may show more thoughts here.
> 
> One concern is the overhead of the DMA API that needs to be benchmarked.
> 
> Thanks

Concern with what? This patch does not change devices, they are using
the existing API. Xuan Zhuo already showed a benchmark result for AF_XDP.


> >
> >
> > >
> > > > 2. The latest Virtio Core supports each rx/tx queue with one DMA device.
> > > >Generally, the physical network card has only one device. All queues 
> > > > use
> > > >it for DMA operation.
> > >
> > > I'm not sure this is a big deal, we just need to use the per virtqueue
> > > dma device to use DMA API.
> >
> > Yes.
> >
> >
> > >
> > > >
> > > > So I consider this problem again, Virtio Core provides only one API.
> > > >
> > > > * virtio_get_dma_dev(queue)
> > > >
> > > > If the return value is NULL, it means that there is no DMA operation. 
> > > > If it is
> > > > not NULL, use DMA API for DMA operation.
> > > >
> > > > The modification of XSK is like this. We may pass NULL as dev to 
> > > > xp_dma_map().
> > > > If dev is NULL, then there is no need to perform DMA and Sync 
> > > > operations.
> > > > Otherwise, it will perform DMA operations like other devices.
> > >
> > > As discussed above, it might be easier:
> > >
> > > if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
> > > return virtio_device;
> > > else
> > > return vring_dma_dev();
> >
> > Yes, according to Jason's opinion, then XSK not need to do any 
> > modifications.
> >
> > Thanks.

Yes AF_XDP does not need the per VQ device hack.
We should probably rethink it.

But as far as implementation goes, poking at VIRTIO_F_ACCESS_PLATFORM
is wrong. Please use virtio_has_dma_quirk.



> > >
> > > >
> > > > And if the dma_dev of rx and tx is different, then we can only disable
> > > > XDP_SOCKET.
> > >
> > > We can start with this.
> > >
> > > Thanks
> > >
> > > >
> > > > Looking forward to everyone's reply.
> > > >
> > > > Thanks.
> > > >
> > > > >
> > > > > Thanks.
> > > > >
> > > > >
> > > > > >
> > > > > > Thanks
> > > > > >
> > > > > > >
> > > > > > >
> > > > > > > >
> > > > > > > > (Otherwise the DMA helpers need to grow/shrink as the DMA API 
> > > > > > > > evolves?)
> > > > > > > >
> > > > > > > > >
> > > > > > > > > Added virtio_dma_unmap() for unmap 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Jason Wang
On Thu, Mar 2, 2023 at 11:24 AM Xuan Zhuo  wrote:
>
> On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  wrote:
> > On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  
> > wrote:
> > >
> > > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo 
> > >  wrote:
> > > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  
> > > > wrote:
> > > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo 
> > > > >  wrote:
> > > > > >
> > > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang 
> > > > > >  wrote:
> > > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > > >  wrote:
> > > > > > > >
> > > > > > > > Added virtio_dma_map() to map DMA addresses for virtual memory 
> > > > > > > > in
> > > > > > > > advance. The purpose is to keep memory mapped across multiple 
> > > > > > > > add/get
> > > > > > > > buf operations.
> > > > > > >
> > > > > > > I wonder if instead of exporting helpers like this, it might be 
> > > > > > > simple
> > > > > > > to just export dma_dev then the upper layer can use DMA API at 
> > > > > > > will?
> > > > > >
> > > > > >
> > > > > > The reason for not doing this, Virtio is not just using DMA_DEV to 
> > > > > > mapp, but
> > > > > > also check whether DMA is used.
> > > > >
> > > > > We should let the DMA API decide by exporting a correct dma_dev. E.g
> > > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> > > > > dma_ops.
> > > >
> > > >
> > > > Do you mean we provide this API?
> > > >
> > > > virtio_get_dma_dev()
> > > >
> > > > If it returns NULL, the caller will use the physical memory address 
> > > > directly. If
> > > > this func return a dma_dev, the caller should use DMA API.
> > >
> > >
> > > cc the XDP_SOCKET's maintainers.
> > >
> > > First of all, Jason does not want to encapsulate the API of DMA by 
> > > Virtio. It is
> > > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP operation 
> > > directly.
> > > I agree with this idea.
> > >
> > > However, there are several problems under Virtio:
> > > 1. In some virtualization scenarios, we do not have to perform DMA 
> > > operations,
> > >just use the physical address directly.
> >
> > This is not a problem, we can simply return the virtio device itself
> > as the DMA device in this case. Since there's no DMA ops attached, DMA
> > API will use physical address in this case.
>
> Is this like this? So why do we have to deal with it in Virtio Ring? Let me
> learn it.

It has a long debate and I can't recall too many details. (You can
search the archives). Michael may show more thoughts here.

One concern is the overhead of the DMA API that needs to be benchmarked.

Thanks

>
>
> >
> > > 2. The latest Virtio Core supports each rx/tx queue with one DMA device.
> > >Generally, the physical network card has only one device. All queues 
> > > use
> > >it for DMA operation.
> >
> > I'm not sure this is a big deal, we just need to use the per virtqueue
> > dma device to use DMA API.
>
> Yes.
>
>
> >
> > >
> > > So I consider this problem again, Virtio Core provides only one API.
> > >
> > > * virtio_get_dma_dev(queue)
> > >
> > > If the return value is NULL, it means that there is no DMA operation. If 
> > > it is
> > > not NULL, use DMA API for DMA operation.
> > >
> > > The modification of XSK is like this. We may pass NULL as dev to 
> > > xp_dma_map().
> > > If dev is NULL, then there is no need to perform DMA and Sync operations.
> > > Otherwise, it will perform DMA operations like other devices.
> >
> > As discussed above, it might be easier:
> >
> > if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
> > return virtio_device;
> > else
> > return vring_dma_dev();
>
> Yes, according to Jason's opinion, then XSK not need to do any modifications.
>
> Thanks.
>
> >
> > >
> > > And if the dma_dev of rx and tx is different, then we can only disable
> > > XDP_SOCKET.
> >
> > We can start with this.
> >
> > Thanks
> >
> > >
> > > Looking forward to everyone's reply.
> > >
> > > Thanks.
> > >
> > > >
> > > > Thanks.
> > > >
> > > >
> > > > >
> > > > > Thanks
> > > > >
> > > > > >
> > > > > >
> > > > > > >
> > > > > > > (Otherwise the DMA helpers need to grow/shrink as the DMA API 
> > > > > > > evolves?)
> > > > > > >
> > > > > > > >
> > > > > > > > Added virtio_dma_unmap() for unmap DMA address.
> > > > > > > >
> > > > > > > > Signed-off-by: Xuan Zhuo 
> > > > > > > > ---
> > > > > > > >  drivers/virtio/virtio_ring.c | 92 
> > > > > > > > 
> > > > > > > >  include/linux/virtio.h   |  9 
> > > > > > > >  2 files changed, 101 insertions(+)
> > > > > > > >
> > > > > > > > diff --git a/drivers/virtio/virtio_ring.c 
> > > > > > > > b/drivers/virtio/virtio_ring.c
> > > > > > > > index cd9364eb2345..855338609c7f 100644
> > > > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > > > @@ -3172,4 +3172,96 @@ const struct vring 
> > > > > > > > *virtqueue_get_vring(struct virtqueue *vq)
> > > > > > 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Xuan Zhuo
On Thu, 2 Mar 2023 11:05:26 +0800, Jason Wang  wrote:
> On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  wrote:
> >
> > On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo  
> > wrote:
> > > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  
> > > wrote:
> > > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  
> > > > wrote:
> > > > >
> > > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  
> > > > > wrote:
> > > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > > >  wrote:
> > > > > > >
> > > > > > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > > > > > advance. The purpose is to keep memory mapped across multiple 
> > > > > > > add/get
> > > > > > > buf operations.
> > > > > >
> > > > > > I wonder if instead of exporting helpers like this, it might be 
> > > > > > simple
> > > > > > to just export dma_dev then the upper layer can use DMA API at will?
> > > > >
> > > > >
> > > > > The reason for not doing this, Virtio is not just using DMA_DEV to 
> > > > > mapp, but
> > > > > also check whether DMA is used.
> > > >
> > > > We should let the DMA API decide by exporting a correct dma_dev. E.g
> > > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> > > > dma_ops.
> > >
> > >
> > > Do you mean we provide this API?
> > >
> > > virtio_get_dma_dev()
> > >
> > > If it returns NULL, the caller will use the physical memory address 
> > > directly. If
> > > this func return a dma_dev, the caller should use DMA API.
> >
> >
> > cc the XDP_SOCKET's maintainers.
> >
> > First of all, Jason does not want to encapsulate the API of DMA by Virtio. 
> > It is
> > best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP operation 
> > directly.
> > I agree with this idea.
> >
> > However, there are several problems under Virtio:
> > 1. In some virtualization scenarios, we do not have to perform DMA 
> > operations,
> >just use the physical address directly.
>
> This is not a problem, we can simply return the virtio device itself
> as the DMA device in this case. Since there's no DMA ops attached, DMA
> API will use physical address in this case.

Is this like this? So why do we have to deal with it in Virtio Ring? Let me
learn it.


>
> > 2. The latest Virtio Core supports each rx/tx queue with one DMA device.
> >Generally, the physical network card has only one device. All queues use
> >it for DMA operation.
>
> I'm not sure this is a big deal, we just need to use the per virtqueue
> dma device to use DMA API.

Yes.


>
> >
> > So I consider this problem again, Virtio Core provides only one API.
> >
> > * virtio_get_dma_dev(queue)
> >
> > If the return value is NULL, it means that there is no DMA operation. If it 
> > is
> > not NULL, use DMA API for DMA operation.
> >
> > The modification of XSK is like this. We may pass NULL as dev to 
> > xp_dma_map().
> > If dev is NULL, then there is no need to perform DMA and Sync operations.
> > Otherwise, it will perform DMA operations like other devices.
>
> As discussed above, it might be easier:
>
> if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
> return virtio_device;
> else
> return vring_dma_dev();

Yes, according to Jason's opinion, then XSK not need to do any modifications.

Thanks.

>
> >
> > And if the dma_dev of rx and tx is different, then we can only disable
> > XDP_SOCKET.
>
> We can start with this.
>
> Thanks
>
> >
> > Looking forward to everyone's reply.
> >
> > Thanks.
> >
> > >
> > > Thanks.
> > >
> > >
> > > >
> > > > Thanks
> > > >
> > > > >
> > > > >
> > > > > >
> > > > > > (Otherwise the DMA helpers need to grow/shrink as the DMA API 
> > > > > > evolves?)
> > > > > >
> > > > > > >
> > > > > > > Added virtio_dma_unmap() for unmap DMA address.
> > > > > > >
> > > > > > > Signed-off-by: Xuan Zhuo 
> > > > > > > ---
> > > > > > >  drivers/virtio/virtio_ring.c | 92 
> > > > > > > 
> > > > > > >  include/linux/virtio.h   |  9 
> > > > > > >  2 files changed, 101 insertions(+)
> > > > > > >
> > > > > > > diff --git a/drivers/virtio/virtio_ring.c 
> > > > > > > b/drivers/virtio/virtio_ring.c
> > > > > > > index cd9364eb2345..855338609c7f 100644
> > > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > > @@ -3172,4 +3172,96 @@ const struct vring 
> > > > > > > *virtqueue_get_vring(struct virtqueue *vq)
> > > > > > >  }
> > > > > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > > > > > >
> > > > > > > +/**
> > > > > > > + * virtio_dma_map_page - get the DMA addr of the memory for 
> > > > > > > virtio device
> > > > > > > + * @dev: virtio device
> > > > > > > + * @page: the page of the memory to DMA
> > > > > > > + * @offset: the offset of the memory inside page
> > > > > > > + * @length: memory length
> > > > > > > + * @dir: DMA direction
> > > > > > > + *
> > > > > > > + * This API is only for pre-mapped buffers, for non premapped 
> > > > > > > buffers virtio
> > > > > > > + * core 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Jason Wang
On Thu, Mar 2, 2023 at 10:24 AM Xuan Zhuo  wrote:
>
> On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo  
> wrote:
> > On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  wrote:
> > > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  
> > > wrote:
> > > >
> > > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  
> > > > wrote:
> > > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo 
> > > > >  wrote:
> > > > > >
> > > > > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > > > > advance. The purpose is to keep memory mapped across multiple 
> > > > > > add/get
> > > > > > buf operations.
> > > > >
> > > > > I wonder if instead of exporting helpers like this, it might be simple
> > > > > to just export dma_dev then the upper layer can use DMA API at will?
> > > >
> > > >
> > > > The reason for not doing this, Virtio is not just using DMA_DEV to 
> > > > mapp, but
> > > > also check whether DMA is used.
> > >
> > > We should let the DMA API decide by exporting a correct dma_dev. E.g
> > > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> > > dma_ops.
> >
> >
> > Do you mean we provide this API?
> >
> > virtio_get_dma_dev()
> >
> > If it returns NULL, the caller will use the physical memory address 
> > directly. If
> > this func return a dma_dev, the caller should use DMA API.
>
>
> cc the XDP_SOCKET's maintainers.
>
> First of all, Jason does not want to encapsulate the API of DMA by Virtio. It 
> is
> best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP operation 
> directly.
> I agree with this idea.
>
> However, there are several problems under Virtio:
> 1. In some virtualization scenarios, we do not have to perform DMA operations,
>just use the physical address directly.

This is not a problem, we can simply return the virtio device itself
as the DMA device in this case. Since there's no DMA ops attached, DMA
API will use physical address in this case.

> 2. The latest Virtio Core supports each rx/tx queue with one DMA device.
>Generally, the physical network card has only one device. All queues use
>it for DMA operation.

I'm not sure this is a big deal, we just need to use the per virtqueue
dma device to use DMA API.

>
> So I consider this problem again, Virtio Core provides only one API.
>
> * virtio_get_dma_dev(queue)
>
> If the return value is NULL, it means that there is no DMA operation. If it is
> not NULL, use DMA API for DMA operation.
>
> The modification of XSK is like this. We may pass NULL as dev to xp_dma_map().
> If dev is NULL, then there is no need to perform DMA and Sync operations.
> Otherwise, it will perform DMA operations like other devices.

As discussed above, it might be easier:

if (!virtio_has_feature(VIRTIO_F_ACCESS_PLATFORM))
return virtio_device;
else
return vring_dma_dev();

>
> And if the dma_dev of rx and tx is different, then we can only disable
> XDP_SOCKET.

We can start with this.

Thanks

>
> Looking forward to everyone's reply.
>
> Thanks.
>
> >
> > Thanks.
> >
> >
> > >
> > > Thanks
> > >
> > > >
> > > >
> > > > >
> > > > > (Otherwise the DMA helpers need to grow/shrink as the DMA API 
> > > > > evolves?)
> > > > >
> > > > > >
> > > > > > Added virtio_dma_unmap() for unmap DMA address.
> > > > > >
> > > > > > Signed-off-by: Xuan Zhuo 
> > > > > > ---
> > > > > >  drivers/virtio/virtio_ring.c | 92 
> > > > > > 
> > > > > >  include/linux/virtio.h   |  9 
> > > > > >  2 files changed, 101 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/virtio/virtio_ring.c 
> > > > > > b/drivers/virtio/virtio_ring.c
> > > > > > index cd9364eb2345..855338609c7f 100644
> > > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > > @@ -3172,4 +3172,96 @@ const struct vring 
> > > > > > *virtqueue_get_vring(struct virtqueue *vq)
> > > > > >  }
> > > > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > > > > >
> > > > > > +/**
> > > > > > + * virtio_dma_map_page - get the DMA addr of the memory for virtio 
> > > > > > device
> > > > > > + * @dev: virtio device
> > > > > > + * @page: the page of the memory to DMA
> > > > > > + * @offset: the offset of the memory inside page
> > > > > > + * @length: memory length
> > > > > > + * @dir: DMA direction
> > > > > > + *
> > > > > > + * This API is only for pre-mapped buffers, for non premapped 
> > > > > > buffers virtio
> > > > > > + * core handles DMA API internally.
> > > > > > + *
> > > > > > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > > > > > + */
> > > > > > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page 
> > > > > > *page, size_t offset,
> > > > > > +  unsigned int length, enum 
> > > > > > dma_data_direction dir)
> > > > > > +{
> > > > >
> > > > > This (and the reset) needs to be done per virtqueue instead per device
> > > > > after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> > > > > virtqueue 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Xuan Zhuo
On Tue, 28 Feb 2023 19:15:23 +0800, Xuan Zhuo  
wrote:
> On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  wrote:
> > On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  
> > wrote:
> > >
> > > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  
> > > wrote:
> > > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  
> > > > wrote:
> > > > >
> > > > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > > > advance. The purpose is to keep memory mapped across multiple add/get
> > > > > buf operations.
> > > >
> > > > I wonder if instead of exporting helpers like this, it might be simple
> > > > to just export dma_dev then the upper layer can use DMA API at will?
> > >
> > >
> > > The reason for not doing this, Virtio is not just using DMA_DEV to mapp, 
> > > but
> > > also check whether DMA is used.
> >
> > We should let the DMA API decide by exporting a correct dma_dev. E.g
> > when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> > dma_ops.
>
>
> Do you mean we provide this API?
>
> virtio_get_dma_dev()
>
> If it returns NULL, the caller will use the physical memory address directly. 
> If
> this func return a dma_dev, the caller should use DMA API.


cc the XDP_SOCKET's maintainers.

First of all, Jason does not want to encapsulate the API of DMA by Virtio. It is
best to pass DMA Device to XSK, XSK uses DMA API for DMA MAP operation directly.
I agree with this idea.

However, there are several problems under Virtio:
1. In some virtualization scenarios, we do not have to perform DMA operations,
   just use the physical address directly.
2. The latest Virtio Core supports each rx/tx queue with one DMA device.
   Generally, the physical network card has only one device. All queues use
   it for DMA operation.

So I consider this problem again, Virtio Core provides only one API.

* virtio_get_dma_dev(queue)

If the return value is NULL, it means that there is no DMA operation. If it is
not NULL, use DMA API for DMA operation.

The modification of XSK is like this. We may pass NULL as dev to xp_dma_map().
If dev is NULL, then there is no need to perform DMA and Sync operations.
Otherwise, it will perform DMA operations like other devices.

And if the dma_dev of rx and tx is different, then we can only disable
XDP_SOCKET.

Looking forward to everyone's reply.

Thanks.

>
> Thanks.
>
>
> >
> > Thanks
> >
> > >
> > >
> > > >
> > > > (Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)
> > > >
> > > > >
> > > > > Added virtio_dma_unmap() for unmap DMA address.
> > > > >
> > > > > Signed-off-by: Xuan Zhuo 
> > > > > ---
> > > > >  drivers/virtio/virtio_ring.c | 92 
> > > > > 
> > > > >  include/linux/virtio.h   |  9 
> > > > >  2 files changed, 101 insertions(+)
> > > > >
> > > > > diff --git a/drivers/virtio/virtio_ring.c 
> > > > > b/drivers/virtio/virtio_ring.c
> > > > > index cd9364eb2345..855338609c7f 100644
> > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> > > > > virtqueue *vq)
> > > > >  }
> > > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > > > >
> > > > > +/**
> > > > > + * virtio_dma_map_page - get the DMA addr of the memory for virtio 
> > > > > device
> > > > > + * @dev: virtio device
> > > > > + * @page: the page of the memory to DMA
> > > > > + * @offset: the offset of the memory inside page
> > > > > + * @length: memory length
> > > > > + * @dir: DMA direction
> > > > > + *
> > > > > + * This API is only for pre-mapped buffers, for non premapped 
> > > > > buffers virtio
> > > > > + * core handles DMA API internally.
> > > > > + *
> > > > > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > > > > + */
> > > > > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page 
> > > > > *page, size_t offset,
> > > > > +  unsigned int length, enum 
> > > > > dma_data_direction dir)
> > > > > +{
> > > >
> > > > This (and the reset) needs to be done per virtqueue instead per device
> > > > after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> > > > virtqueue dma device").
> > >
> > >
> > > YES.
> > >
> > >
> > > >
> > > > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > > > +
> > > > > +   if (!vring_use_dma_api(vdev))
> > > > > +   return page_to_phys(page) + offset;
> > > > > +
> > > > > +   return dma_map_page(vdev->dev.parent, page, offset, length, 
> > > > > dir);
> > > > > +}
> > > >
> > > > Need either inline or EXPORT_SYMBOL_GPL() here.
> > >
> > > Because I did not use this interface, I did not  export it.
> > >
> > > Thanks.
> > >
> > >
> > > >
> > > > Thanks
> > > >
> > > >
> > > > > +
> > > > > +/**
> > > > > + * virtio_dma_map - get the DMA addr of the memory for virtio device
> > > > > + * @dev: virtio device
> > > > > + * @addr: the addr to DMA
> > > > > + * @length: memory length
> > > > > + * @dir: 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-03-01 Thread Xuan Zhuo
On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  wrote:
> On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  wrote:
> >
> > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  wrote:
> > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  
> > > wrote:
> > > >
> > > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > > advance. The purpose is to keep memory mapped across multiple add/get
> > > > buf operations.
> > >
> > > I wonder if instead of exporting helpers like this, it might be simple
> > > to just export dma_dev then the upper layer can use DMA API at will?
> >
> >
> > The reason for not doing this, Virtio is not just using DMA_DEV to mapp, but
> > also check whether DMA is used.
>
> We should let the DMA API decide by exporting a correct dma_dev. E.g
> when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> dma_ops.


In XSK, we need to pass a device to XSK.

If vdev->dev is passed, we can't get dma_dev in XSK. Because each VQ has a
dma_dev. So we should pass dma_dev to XSK. But how do we determine whether to
use DMA_OPS based on dma_dev?

At present, my API design, the caller should determine whether it is a Virtio
device. If we also need the caller to determine whether to use DMA_OPS, this is
too unfriendly for the caller.

Thanks.


>
> Thanks
>
> >
> >
> > >
> > > (Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)
> > >
> > > >
> > > > Added virtio_dma_unmap() for unmap DMA address.
> > > >
> > > > Signed-off-by: Xuan Zhuo 
> > > > ---
> > > >  drivers/virtio/virtio_ring.c | 92 
> > > >  include/linux/virtio.h   |  9 
> > > >  2 files changed, 101 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd9364eb2345..855338609c7f 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> > > > virtqueue *vq)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > > >
> > > > +/**
> > > > + * virtio_dma_map_page - get the DMA addr of the memory for virtio 
> > > > device
> > > > + * @dev: virtio device
> > > > + * @page: the page of the memory to DMA
> > > > + * @offset: the offset of the memory inside page
> > > > + * @length: memory length
> > > > + * @dir: DMA direction
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > > > + */
> > > > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, 
> > > > size_t offset,
> > > > +  unsigned int length, enum 
> > > > dma_data_direction dir)
> > > > +{
> > >
> > > This (and the reset) needs to be done per virtqueue instead per device
> > > after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> > > virtqueue dma device").
> >
> >
> > YES.
> >
> >
> > >
> > > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > > +
> > > > +   if (!vring_use_dma_api(vdev))
> > > > +   return page_to_phys(page) + offset;
> > > > +
> > > > +   return dma_map_page(vdev->dev.parent, page, offset, length, 
> > > > dir);
> > > > +}
> > >
> > > Need either inline or EXPORT_SYMBOL_GPL() here.
> >
> > Because I did not use this interface, I did not  export it.
> >
> > Thanks.
> >
> >
> > >
> > > Thanks
> > >
> > >
> > > > +
> > > > +/**
> > > > + * virtio_dma_map - get the DMA addr of the memory for virtio device
> > > > + * @dev: virtio device
> > > > + * @addr: the addr to DMA
> > > > + * @length: memory length
> > > > + * @dir: DMA direction
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns the DMA addr.
> > > > + */
> > > > +dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int 
> > > > length,
> > > > + enum dma_data_direction dir)
> > > > +{
> > > > +   struct page *page;
> > > > +   size_t offset;
> > > > +
> > > > +   page = virt_to_page(addr);
> > > > +   offset = offset_in_page(addr);
> > > > +
> > > > +   return virtio_dma_map_page(dev, page, offset, length, dir);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(virtio_dma_map);
> > > > +
> > > > +/**
> > > > + * virtio_dma_mapping_error - check dma address
> > > > + * @dev: virtio device
> > > > + * @addr: DMA address
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns 0 means dma valid. Other means invalid dma address.
> > > > + */
> > > > +int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
> > > > +{
> > > > +   struct virtio_device *vdev = 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-02-28 Thread Xuan Zhuo
On Tue, 21 Feb 2023 09:51:07 +0800, Jason Wang  wrote:
> On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  wrote:
> >
> > On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  wrote:
> > > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  
> > > wrote:
> > > >
> > > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > > advance. The purpose is to keep memory mapped across multiple add/get
> > > > buf operations.
> > >
> > > I wonder if instead of exporting helpers like this, it might be simple
> > > to just export dma_dev then the upper layer can use DMA API at will?
> >
> >
> > The reason for not doing this, Virtio is not just using DMA_DEV to mapp, but
> > also check whether DMA is used.
>
> We should let the DMA API decide by exporting a correct dma_dev. E.g
> when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
> dma_ops.


Do you mean we provide this API?

virtio_get_dma_dev()

If it returns NULL, the caller will use the physical memory address directly. If
this func return a dma_dev, the caller should use DMA API.

Thanks.


>
> Thanks
>
> >
> >
> > >
> > > (Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)
> > >
> > > >
> > > > Added virtio_dma_unmap() for unmap DMA address.
> > > >
> > > > Signed-off-by: Xuan Zhuo 
> > > > ---
> > > >  drivers/virtio/virtio_ring.c | 92 
> > > >  include/linux/virtio.h   |  9 
> > > >  2 files changed, 101 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > index cd9364eb2345..855338609c7f 100644
> > > > --- a/drivers/virtio/virtio_ring.c
> > > > +++ b/drivers/virtio/virtio_ring.c
> > > > @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> > > > virtqueue *vq)
> > > >  }
> > > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > > >
> > > > +/**
> > > > + * virtio_dma_map_page - get the DMA addr of the memory for virtio 
> > > > device
> > > > + * @dev: virtio device
> > > > + * @page: the page of the memory to DMA
> > > > + * @offset: the offset of the memory inside page
> > > > + * @length: memory length
> > > > + * @dir: DMA direction
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > > > + */
> > > > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, 
> > > > size_t offset,
> > > > +  unsigned int length, enum 
> > > > dma_data_direction dir)
> > > > +{
> > >
> > > This (and the reset) needs to be done per virtqueue instead per device
> > > after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> > > virtqueue dma device").
> >
> >
> > YES.
> >
> >
> > >
> > > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > > +
> > > > +   if (!vring_use_dma_api(vdev))
> > > > +   return page_to_phys(page) + offset;
> > > > +
> > > > +   return dma_map_page(vdev->dev.parent, page, offset, length, 
> > > > dir);
> > > > +}
> > >
> > > Need either inline or EXPORT_SYMBOL_GPL() here.
> >
> > Because I did not use this interface, I did not  export it.
> >
> > Thanks.
> >
> >
> > >
> > > Thanks
> > >
> > >
> > > > +
> > > > +/**
> > > > + * virtio_dma_map - get the DMA addr of the memory for virtio device
> > > > + * @dev: virtio device
> > > > + * @addr: the addr to DMA
> > > > + * @length: memory length
> > > > + * @dir: DMA direction
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns the DMA addr.
> > > > + */
> > > > +dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int 
> > > > length,
> > > > + enum dma_data_direction dir)
> > > > +{
> > > > +   struct page *page;
> > > > +   size_t offset;
> > > > +
> > > > +   page = virt_to_page(addr);
> > > > +   offset = offset_in_page(addr);
> > > > +
> > > > +   return virtio_dma_map_page(dev, page, offset, length, dir);
> > > > +}
> > > > +EXPORT_SYMBOL_GPL(virtio_dma_map);
> > > > +
> > > > +/**
> > > > + * virtio_dma_mapping_error - check dma address
> > > > + * @dev: virtio device
> > > > + * @addr: DMA address
> > > > + *
> > > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > > virtio
> > > > + * core handles DMA API internally.
> > > > + *
> > > > + * Returns 0 means dma valid. Other means invalid dma address.
> > > > + */
> > > > +int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
> > > > +{
> > > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > > +
> > > > +   if (!vring_use_dma_api(vdev))
> > > > +   return 0;
> > > > +
> > > > +   return dma_mapping_error(vdev->dev.parent, addr);
> > > > +}
> > > > 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-02-20 Thread Jason Wang
On Mon, Feb 20, 2023 at 3:02 PM Xuan Zhuo  wrote:
>
> On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  wrote:
> > On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  
> > wrote:
> > >
> > > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > > advance. The purpose is to keep memory mapped across multiple add/get
> > > buf operations.
> >
> > I wonder if instead of exporting helpers like this, it might be simple
> > to just export dma_dev then the upper layer can use DMA API at will?
>
>
> The reason for not doing this, Virtio is not just using DMA_DEV to mapp, but
> also check whether DMA is used.

We should let the DMA API decide by exporting a correct dma_dev. E.g
when ACCESS_PLATFORM is not negotiated, advertising a DMA dev without
dma_ops.

Thanks

>
>
> >
> > (Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)
> >
> > >
> > > Added virtio_dma_unmap() for unmap DMA address.
> > >
> > > Signed-off-by: Xuan Zhuo 
> > > ---
> > >  drivers/virtio/virtio_ring.c | 92 
> > >  include/linux/virtio.h   |  9 
> > >  2 files changed, 101 insertions(+)
> > >
> > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > index cd9364eb2345..855338609c7f 100644
> > > --- a/drivers/virtio/virtio_ring.c
> > > +++ b/drivers/virtio/virtio_ring.c
> > > @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> > > virtqueue *vq)
> > >  }
> > >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> > >
> > > +/**
> > > + * virtio_dma_map_page - get the DMA addr of the memory for virtio device
> > > + * @dev: virtio device
> > > + * @page: the page of the memory to DMA
> > > + * @offset: the offset of the memory inside page
> > > + * @length: memory length
> > > + * @dir: DMA direction
> > > + *
> > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > virtio
> > > + * core handles DMA API internally.
> > > + *
> > > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > > + */
> > > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, 
> > > size_t offset,
> > > +  unsigned int length, enum 
> > > dma_data_direction dir)
> > > +{
> >
> > This (and the reset) needs to be done per virtqueue instead per device
> > after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> > virtqueue dma device").
>
>
> YES.
>
>
> >
> > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > +
> > > +   if (!vring_use_dma_api(vdev))
> > > +   return page_to_phys(page) + offset;
> > > +
> > > +   return dma_map_page(vdev->dev.parent, page, offset, length, dir);
> > > +}
> >
> > Need either inline or EXPORT_SYMBOL_GPL() here.
>
> Because I did not use this interface, I did not  export it.
>
> Thanks.
>
>
> >
> > Thanks
> >
> >
> > > +
> > > +/**
> > > + * virtio_dma_map - get the DMA addr of the memory for virtio device
> > > + * @dev: virtio device
> > > + * @addr: the addr to DMA
> > > + * @length: memory length
> > > + * @dir: DMA direction
> > > + *
> > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > virtio
> > > + * core handles DMA API internally.
> > > + *
> > > + * Returns the DMA addr.
> > > + */
> > > +dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int 
> > > length,
> > > + enum dma_data_direction dir)
> > > +{
> > > +   struct page *page;
> > > +   size_t offset;
> > > +
> > > +   page = virt_to_page(addr);
> > > +   offset = offset_in_page(addr);
> > > +
> > > +   return virtio_dma_map_page(dev, page, offset, length, dir);
> > > +}
> > > +EXPORT_SYMBOL_GPL(virtio_dma_map);
> > > +
> > > +/**
> > > + * virtio_dma_mapping_error - check dma address
> > > + * @dev: virtio device
> > > + * @addr: DMA address
> > > + *
> > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > virtio
> > > + * core handles DMA API internally.
> > > + *
> > > + * Returns 0 means dma valid. Other means invalid dma address.
> > > + */
> > > +int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
> > > +{
> > > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > > +
> > > +   if (!vring_use_dma_api(vdev))
> > > +   return 0;
> > > +
> > > +   return dma_mapping_error(vdev->dev.parent, addr);
> > > +}
> > > +EXPORT_SYMBOL_GPL(virtio_dma_mapping_error);
> > > +
> > > +/**
> > > + * virtio_dma_unmap - unmap DMA addr
> > > + * @dev: virtio device
> > > + * @dma: DMA address
> > > + * @length: memory length
> > > + * @dir: DMA direction
> > > + *
> > > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > > virtio
> > > + * core handles DMA API internally.
> > > + */
> > > +void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int 
> > > length,
> > > + enum dma_data_direction dir)
> > > +{
> > > +   struct virtio_device *vdev = 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-02-19 Thread Xuan Zhuo
On Mon, 20 Feb 2023 13:38:24 +0800, Jason Wang  wrote:
> On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  wrote:
> >
> > Added virtio_dma_map() to map DMA addresses for virtual memory in
> > advance. The purpose is to keep memory mapped across multiple add/get
> > buf operations.
>
> I wonder if instead of exporting helpers like this, it might be simple
> to just export dma_dev then the upper layer can use DMA API at will?


The reason for not doing this, Virtio is not just using DMA_DEV to mapp, but
also check whether DMA is used.


>
> (Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)
>
> >
> > Added virtio_dma_unmap() for unmap DMA address.
> >
> > Signed-off-by: Xuan Zhuo 
> > ---
> >  drivers/virtio/virtio_ring.c | 92 
> >  include/linux/virtio.h   |  9 
> >  2 files changed, 101 insertions(+)
> >
> > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > index cd9364eb2345..855338609c7f 100644
> > --- a/drivers/virtio/virtio_ring.c
> > +++ b/drivers/virtio/virtio_ring.c
> > @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> > virtqueue *vq)
> >  }
> >  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
> >
> > +/**
> > + * virtio_dma_map_page - get the DMA addr of the memory for virtio device
> > + * @dev: virtio device
> > + * @page: the page of the memory to DMA
> > + * @offset: the offset of the memory inside page
> > + * @length: memory length
> > + * @dir: DMA direction
> > + *
> > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > virtio
> > + * core handles DMA API internally.
> > + *
> > + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> > + */
> > +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, 
> > size_t offset,
> > +  unsigned int length, enum dma_data_direction 
> > dir)
> > +{
>
> This (and the reset) needs to be done per virtqueue instead per device
> after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
> virtqueue dma device").


YES.


>
> > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > +
> > +   if (!vring_use_dma_api(vdev))
> > +   return page_to_phys(page) + offset;
> > +
> > +   return dma_map_page(vdev->dev.parent, page, offset, length, dir);
> > +}
>
> Need either inline or EXPORT_SYMBOL_GPL() here.

Because I did not use this interface, I did not  export it.

Thanks.


>
> Thanks
>
>
> > +
> > +/**
> > + * virtio_dma_map - get the DMA addr of the memory for virtio device
> > + * @dev: virtio device
> > + * @addr: the addr to DMA
> > + * @length: memory length
> > + * @dir: DMA direction
> > + *
> > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > virtio
> > + * core handles DMA API internally.
> > + *
> > + * Returns the DMA addr.
> > + */
> > +dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int 
> > length,
> > + enum dma_data_direction dir)
> > +{
> > +   struct page *page;
> > +   size_t offset;
> > +
> > +   page = virt_to_page(addr);
> > +   offset = offset_in_page(addr);
> > +
> > +   return virtio_dma_map_page(dev, page, offset, length, dir);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_dma_map);
> > +
> > +/**
> > + * virtio_dma_mapping_error - check dma address
> > + * @dev: virtio device
> > + * @addr: DMA address
> > + *
> > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > virtio
> > + * core handles DMA API internally.
> > + *
> > + * Returns 0 means dma valid. Other means invalid dma address.
> > + */
> > +int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
> > +{
> > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > +
> > +   if (!vring_use_dma_api(vdev))
> > +   return 0;
> > +
> > +   return dma_mapping_error(vdev->dev.parent, addr);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_dma_mapping_error);
> > +
> > +/**
> > + * virtio_dma_unmap - unmap DMA addr
> > + * @dev: virtio device
> > + * @dma: DMA address
> > + * @length: memory length
> > + * @dir: DMA direction
> > + *
> > + * This API is only for pre-mapped buffers, for non premapped buffers 
> > virtio
> > + * core handles DMA API internally.
> > + */
> > +void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int 
> > length,
> > + enum dma_data_direction dir)
> > +{
> > +   struct virtio_device *vdev = dev_to_virtio(dev);
> > +
> > +   if (!vring_use_dma_api(vdev))
> > +   return;
> > +
> > +   dma_unmap_page(vdev->dev.parent, dma, length, dir);
> > +}
> > +EXPORT_SYMBOL_GPL(virtio_dma_unmap);
> > +
> >  MODULE_LICENSE("GPL");
> > diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> > index 3ebb346ebb7c..b5fa71476737 100644
> > --- a/include/linux/virtio.h
> > +++ b/include/linux/virtio.h
> > @@ -9,6 +9,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >
> > 

Re: [PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-02-19 Thread Jason Wang
On Tue, Feb 14, 2023 at 3:27 PM Xuan Zhuo  wrote:
>
> Added virtio_dma_map() to map DMA addresses for virtual memory in
> advance. The purpose is to keep memory mapped across multiple add/get
> buf operations.

I wonder if instead of exporting helpers like this, it might be simple
to just export dma_dev then the upper layer can use DMA API at will?

(Otherwise the DMA helpers need to grow/shrink as the DMA API evolves?)

>
> Added virtio_dma_unmap() for unmap DMA address.
>
> Signed-off-by: Xuan Zhuo 
> ---
>  drivers/virtio/virtio_ring.c | 92 
>  include/linux/virtio.h   |  9 
>  2 files changed, 101 insertions(+)
>
> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> index cd9364eb2345..855338609c7f 100644
> --- a/drivers/virtio/virtio_ring.c
> +++ b/drivers/virtio/virtio_ring.c
> @@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct 
> virtqueue *vq)
>  }
>  EXPORT_SYMBOL_GPL(virtqueue_get_vring);
>
> +/**
> + * virtio_dma_map_page - get the DMA addr of the memory for virtio device
> + * @dev: virtio device
> + * @page: the page of the memory to DMA
> + * @offset: the offset of the memory inside page
> + * @length: memory length
> + * @dir: DMA direction
> + *
> + * This API is only for pre-mapped buffers, for non premapped buffers virtio
> + * core handles DMA API internally.
> + *
> + * Returns the DMA addr. DMA_MAPPING_ERROR means error.
> + */
> +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t 
> offset,
> +  unsigned int length, enum dma_data_direction 
> dir)
> +{

This (and the reset) needs to be done per virtqueue instead per device
after b0e504e5505d184b0be248b7dcdbe50b79f03758 ("virtio_ring: per
virtqueue dma device").

> +   struct virtio_device *vdev = dev_to_virtio(dev);
> +
> +   if (!vring_use_dma_api(vdev))
> +   return page_to_phys(page) + offset;
> +
> +   return dma_map_page(vdev->dev.parent, page, offset, length, dir);
> +}

Need either inline or EXPORT_SYMBOL_GPL() here.

Thanks


> +
> +/**
> + * virtio_dma_map - get the DMA addr of the memory for virtio device
> + * @dev: virtio device
> + * @addr: the addr to DMA
> + * @length: memory length
> + * @dir: DMA direction
> + *
> + * This API is only for pre-mapped buffers, for non premapped buffers virtio
> + * core handles DMA API internally.
> + *
> + * Returns the DMA addr.
> + */
> +dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int 
> length,
> + enum dma_data_direction dir)
> +{
> +   struct page *page;
> +   size_t offset;
> +
> +   page = virt_to_page(addr);
> +   offset = offset_in_page(addr);
> +
> +   return virtio_dma_map_page(dev, page, offset, length, dir);
> +}
> +EXPORT_SYMBOL_GPL(virtio_dma_map);
> +
> +/**
> + * virtio_dma_mapping_error - check dma address
> + * @dev: virtio device
> + * @addr: DMA address
> + *
> + * This API is only for pre-mapped buffers, for non premapped buffers virtio
> + * core handles DMA API internally.
> + *
> + * Returns 0 means dma valid. Other means invalid dma address.
> + */
> +int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
> +{
> +   struct virtio_device *vdev = dev_to_virtio(dev);
> +
> +   if (!vring_use_dma_api(vdev))
> +   return 0;
> +
> +   return dma_mapping_error(vdev->dev.parent, addr);
> +}
> +EXPORT_SYMBOL_GPL(virtio_dma_mapping_error);
> +
> +/**
> + * virtio_dma_unmap - unmap DMA addr
> + * @dev: virtio device
> + * @dma: DMA address
> + * @length: memory length
> + * @dir: DMA direction
> + *
> + * This API is only for pre-mapped buffers, for non premapped buffers virtio
> + * core handles DMA API internally.
> + */
> +void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int 
> length,
> + enum dma_data_direction dir)
> +{
> +   struct virtio_device *vdev = dev_to_virtio(dev);
> +
> +   if (!vring_use_dma_api(vdev))
> +   return;
> +
> +   dma_unmap_page(vdev->dev.parent, dma, length, dir);
> +}
> +EXPORT_SYMBOL_GPL(virtio_dma_unmap);
> +
>  MODULE_LICENSE("GPL");
> diff --git a/include/linux/virtio.h b/include/linux/virtio.h
> index 3ebb346ebb7c..b5fa71476737 100644
> --- a/include/linux/virtio.h
> +++ b/include/linux/virtio.h
> @@ -9,6 +9,7 @@
>  #include 
>  #include 
>  #include 
> +#include 
>
>  /**
>   * struct virtqueue - a queue to register buffers for sending or receiving.
> @@ -216,4 +217,12 @@ void unregister_virtio_driver(struct virtio_driver *drv);
>  #define module_virtio_driver(__virtio_driver) \
> module_driver(__virtio_driver, register_virtio_driver, \
> unregister_virtio_driver)
> +
> +dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t 
> offset,
> +  unsigned int length, enum dma_data_direction 
> dir);
> +dma_addr_t virtio_dma_map(struct 

[PATCH vhost 07/10] virtio_ring: add api virtio_dma_map() for advance dma

2023-02-13 Thread Xuan Zhuo
Added virtio_dma_map() to map DMA addresses for virtual memory in
advance. The purpose is to keep memory mapped across multiple add/get
buf operations.

Added virtio_dma_unmap() for unmap DMA address.

Signed-off-by: Xuan Zhuo 
---
 drivers/virtio/virtio_ring.c | 92 
 include/linux/virtio.h   |  9 
 2 files changed, 101 insertions(+)

diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
index cd9364eb2345..855338609c7f 100644
--- a/drivers/virtio/virtio_ring.c
+++ b/drivers/virtio/virtio_ring.c
@@ -3172,4 +3172,96 @@ const struct vring *virtqueue_get_vring(struct virtqueue 
*vq)
 }
 EXPORT_SYMBOL_GPL(virtqueue_get_vring);
 
+/**
+ * virtio_dma_map_page - get the DMA addr of the memory for virtio device
+ * @dev: virtio device
+ * @page: the page of the memory to DMA
+ * @offset: the offset of the memory inside page
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns the DMA addr. DMA_MAPPING_ERROR means error.
+ */
+dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t 
offset,
+  unsigned int length, enum dma_data_direction dir)
+{
+   struct virtio_device *vdev = dev_to_virtio(dev);
+
+   if (!vring_use_dma_api(vdev))
+   return page_to_phys(page) + offset;
+
+   return dma_map_page(vdev->dev.parent, page, offset, length, dir);
+}
+
+/**
+ * virtio_dma_map - get the DMA addr of the memory for virtio device
+ * @dev: virtio device
+ * @addr: the addr to DMA
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns the DMA addr.
+ */
+dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int length,
+ enum dma_data_direction dir)
+{
+   struct page *page;
+   size_t offset;
+
+   page = virt_to_page(addr);
+   offset = offset_in_page(addr);
+
+   return virtio_dma_map_page(dev, page, offset, length, dir);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_map);
+
+/**
+ * virtio_dma_mapping_error - check dma address
+ * @dev: virtio device
+ * @addr: DMA address
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ *
+ * Returns 0 means dma valid. Other means invalid dma address.
+ */
+int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr)
+{
+   struct virtio_device *vdev = dev_to_virtio(dev);
+
+   if (!vring_use_dma_api(vdev))
+   return 0;
+
+   return dma_mapping_error(vdev->dev.parent, addr);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_mapping_error);
+
+/**
+ * virtio_dma_unmap - unmap DMA addr
+ * @dev: virtio device
+ * @dma: DMA address
+ * @length: memory length
+ * @dir: DMA direction
+ *
+ * This API is only for pre-mapped buffers, for non premapped buffers virtio
+ * core handles DMA API internally.
+ */
+void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int length,
+ enum dma_data_direction dir)
+{
+   struct virtio_device *vdev = dev_to_virtio(dev);
+
+   if (!vring_use_dma_api(vdev))
+   return;
+
+   dma_unmap_page(vdev->dev.parent, dma, length, dir);
+}
+EXPORT_SYMBOL_GPL(virtio_dma_unmap);
+
 MODULE_LICENSE("GPL");
diff --git a/include/linux/virtio.h b/include/linux/virtio.h
index 3ebb346ebb7c..b5fa71476737 100644
--- a/include/linux/virtio.h
+++ b/include/linux/virtio.h
@@ -9,6 +9,7 @@
 #include 
 #include 
 #include 
+#include 
 
 /**
  * struct virtqueue - a queue to register buffers for sending or receiving.
@@ -216,4 +217,12 @@ void unregister_virtio_driver(struct virtio_driver *drv);
 #define module_virtio_driver(__virtio_driver) \
module_driver(__virtio_driver, register_virtio_driver, \
unregister_virtio_driver)
+
+dma_addr_t virtio_dma_map_page(struct device *dev, struct page *page, size_t 
offset,
+  unsigned int length, enum dma_data_direction 
dir);
+dma_addr_t virtio_dma_map(struct device *dev, void *addr, unsigned int length,
+ enum dma_data_direction dir);
+int virtio_dma_mapping_error(struct device *dev, dma_addr_t addr);
+void virtio_dma_unmap(struct device *dev, dma_addr_t dma, unsigned int length,
+ enum dma_data_direction dir);
 #endif /* _LINUX_VIRTIO_H */
-- 
2.32.0.3.g01195cf9f

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization