thr3ads.net - Linux Virtualization - [PATCH 3/3] virtio_pci: Use the DMA API for virtqueues [Aug 2014]

If this information is useful, please help other people find it:
Share via:

Konrad Rzeszutek Wilk

2014-Aug-27 17:32 UTC

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

On Tue, Aug 26, 2014 at 02:17:02PM -0700, Andy Lutomirski
wrote:> A virtqueue is a coherent DMA mapping.  Use the DMA API for it.
> This fixes virtio_pci on Xen.
> 
> Signed-off-by: Andy Lutomirski <luto at amacapital.net>
> ---
>  drivers/virtio/virtio_pci.c | 25 ++++++++++++++++++-------
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
> index 3d1463c6b120..19039c5bec24 100644
> --- a/drivers/virtio/virtio_pci.c
> +++ b/drivers/virtio/virtio_pci.c
> @@ -80,8 +80,9 @@ struct virtio_pci_vq_info
>  	/* the number of entries in the queue */
>  	int num;
>  
> -	/* the virtual address of the ring queue */
> -	void *queue;
> +	/* the ring queue */
> +	void *queue;			/* virtual address */
> +	dma_addr_t queue_dma_addr;	/* bus address */
>  
>  	/* the list node for the virtqueues list */
>  	struct list_head node;
> @@ -417,15 +418,16 @@ static struct virtqueue *setup_vq(struct
virtio_device *vdev, unsigned index,
>  	info->num = num;
>  	info->msix_vector = msix_vec;
>  
> -	size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
> -	info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
> +	size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
> +	info->queue = dma_zalloc_coherent(vdev->dev.parent, size,
> +					  &info->queue_dma_addr, GFP_KERNEL);
>  	if (info->queue == NULL) {
>  		err = -ENOMEM;
>  		goto out_info;
>  	}
>  
>  	/* activate the queue */
> -	iowrite32(virt_to_phys(info->queue) >>
VIRTIO_PCI_QUEUE_ADDR_SHIFT,
> +	iowrite32(info->queue_dma_addr >> VIRTIO_PCI_QUEUE_ADDR_SHIFT,
>  		  vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
>  
>  	/* create the vring */
> @@ -462,7 +464,8 @@ out_assign:
>  	vring_del_virtqueue(vq);
>  out_activate_queue:
>  	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
> -	free_pages_exact(info->queue, size);
> +	dma_free_coherent(vdev->dev.parent, size,
> +			  info->queue, info->queue_dma_addr);
>  out_info:
>  	kfree(info);
>  	return ERR_PTR(err);
> @@ -493,7 +496,8 @@ static void vp_del_vq(struct virtqueue *vq)
>  	iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
>  
>  	size = PAGE_ALIGN(vring_size(info->num, VIRTIO_PCI_VRING_ALIGN));
> -	free_pages_exact(info->queue, size);
> +	dma_free_coherent(vq->vdev->dev.parent, size,
> +			  info->queue, info->queue_dma_addr);
>  	kfree(info);
>  }
>  
> @@ -712,6 +716,13 @@ static int virtio_pci_probe(struct pci_dev *pci_dev,
>  	if (err)
>  		goto out;
>  
> +	/*
> +	 * We support 64-bit DMA.  If this fails (e.g. some bridge
> +	 * or PV code doesn't or DAC is disabled), then we're okay
> +	 * with 32-bit DMA.
<scratches his head>

I am having a hard time parsing that. Could you expand a bit the
faulting use-case please?
> +	 */
> +	dma_set_mask_and_coherent(&pci_dev->dev, DMA_BIT_MASK(64));
The usual process is:

	ret = dma_set_mask_and_coherent(..)
	if (ret)
		ret = dma_set_mask_and_coherent(.., DMA_BIT_MASK(32))

	if (ret)
		pr_warn("We are truly screwed. Good luck!\n");
> +
>  	err = pci_request_regions(pci_dev, "virtio-pci");
>  	if (err)
>  		goto out_enable_device;
> -- 
> 1.9.3
>

Andy Lutomirski

2014-Aug-27 17:35 UTC

head link

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

On Wed, Aug 27, 2014 at 10:32 AM, Konrad Rzeszutek Wilk
<konrad.wilk at oracle.com> wrote:> On Tue, Aug 26, 2014 at 02:17:02PM -0700, Andy Lutomirski wrote:
>> A virtqueue is a coherent DMA mapping.  Use the DMA API for it.
>> This fixes virtio_pci on Xen.
>>
>> Signed-off-by: Andy Lutomirski <luto at amacapital.net>
>> ---
>>  drivers/virtio/virtio_pci.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
>> index 3d1463c6b120..19039c5bec24 100644
>> --- a/drivers/virtio/virtio_pci.c
>> +++ b/drivers/virtio/virtio_pci.c
>> @@ -80,8 +80,9 @@ struct virtio_pci_vq_info
>>       /* the number of entries in the queue */
>>       int num;
>>
>> -     /* the virtual address of the ring queue */
>> -     void *queue;
>> +     /* the ring queue */
>> +     void *queue;                    /* virtual address */
>> +     dma_addr_t queue_dma_addr;      /* bus address */
>>
>>       /* the list node for the virtqueues list */
>>       struct list_head node;
>> @@ -417,15 +418,16 @@ static struct virtqueue *setup_vq(struct
virtio_device *vdev, unsigned index,
>>       info->num = num;
>>       info->msix_vector = msix_vec;
>>
>> -     size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
>> -     info->queue = alloc_pages_exact(size, GFP_KERNEL|__GFP_ZERO);
>> +     size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
>> +     info->queue = dma_zalloc_coherent(vdev->dev.parent, size,
>> +                                       &info->queue_dma_addr,
GFP_KERNEL);
>>       if (info->queue == NULL) {
>>               err = -ENOMEM;
>>               goto out_info;
>>       }
>>
>>       /* activate the queue */
>> -     iowrite32(virt_to_phys(info->queue) >>
VIRTIO_PCI_QUEUE_ADDR_SHIFT,
>> +     iowrite32(info->queue_dma_addr >>
VIRTIO_PCI_QUEUE_ADDR_SHIFT,
>>                 vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
>>
>>       /* create the vring */
>> @@ -462,7 +464,8 @@ out_assign:
>>       vring_del_virtqueue(vq);
>>  out_activate_queue:
>>       iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
>> -     free_pages_exact(info->queue, size);
>> +     dma_free_coherent(vdev->dev.parent, size,
>> +                       info->queue, info->queue_dma_addr);
>>  out_info:
>>       kfree(info);
>>       return ERR_PTR(err);
>> @@ -493,7 +496,8 @@ static void vp_del_vq(struct virtqueue *vq)
>>       iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
>>
>>       size = PAGE_ALIGN(vring_size(info->num,
VIRTIO_PCI_VRING_ALIGN));
>> -     free_pages_exact(info->queue, size);
>> +     dma_free_coherent(vq->vdev->dev.parent, size,
>> +                       info->queue, info->queue_dma_addr);
>>       kfree(info);
>>  }
>>
>> @@ -712,6 +716,13 @@ static int virtio_pci_probe(struct pci_dev
*pci_dev,
>>       if (err)
>>               goto out;
>>
>> +     /*
>> +      * We support 64-bit DMA.  If this fails (e.g. some bridge
>> +      * or PV code doesn't or DAC is disabled), then we're
okay
>> +      * with 32-bit DMA.
>
> <scratches his head>
>
> I am having a hard time parsing that. Could you expand a bit the
> faulting use-case please?
>
>> +      */
>> +     dma_set_mask_and_coherent(&pci_dev->dev,
DMA_BIT_MASK(64));
>
> The usual process is:
>
>         ret = dma_set_mask_and_coherent(..)
>         if (ret)
>                 ret = dma_set_mask_and_coherent(.., DMA_BIT_MASK(32))
>
>         if (ret)
>                 pr_warn("We are truly screwed. Good luck!\n");
>
I assumed that, if dma_set_mask_and_coherent(..., DMA_BIT_BASK(64))
fails, then we can still do DMA, just not 64-bit DMA.  This driver
should be fine with that -- it'll just be a bit slower.

If that's not a safe assumption, I can change it.

--Andy

Konrad Rzeszutek Wilk

2014-Aug-27 18:52 UTC

head link

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

On Wed, Aug 27, 2014 at 10:35:10AM -0700, Andy Lutomirski
wrote:> On Wed, Aug 27, 2014 at 10:32 AM, Konrad Rzeszutek Wilk
> <konrad.wilk at oracle.com> wrote:
> > On Tue, Aug 26, 2014 at 02:17:02PM -0700, Andy Lutomirski wrote:
> >> A virtqueue is a coherent DMA mapping.  Use the DMA API for it.
> >> This fixes virtio_pci on Xen.
> >>
> >> Signed-off-by: Andy Lutomirski <luto at amacapital.net>
> >> ---
> >>  drivers/virtio/virtio_pci.c | 25 ++++++++++++++++++-------
> >>  1 file changed, 18 insertions(+), 7 deletions(-)
> >>
> >> diff --git a/drivers/virtio/virtio_pci.c
b/drivers/virtio/virtio_pci.c
> >> index 3d1463c6b120..19039c5bec24 100644
> >> --- a/drivers/virtio/virtio_pci.c
> >> +++ b/drivers/virtio/virtio_pci.c
> >> @@ -80,8 +80,9 @@ struct virtio_pci_vq_info
> >>       /* the number of entries in the queue */
> >>       int num;
> >>
> >> -     /* the virtual address of the ring queue */
> >> -     void *queue;
> >> +     /* the ring queue */
> >> +     void *queue;                    /* virtual address */
> >> +     dma_addr_t queue_dma_addr;      /* bus address */
> >>
> >>       /* the list node for the virtqueues list */
> >>       struct list_head node;
> >> @@ -417,15 +418,16 @@ static struct virtqueue *setup_vq(struct
virtio_device *vdev, unsigned index,
> >>       info->num = num;
> >>       info->msix_vector = msix_vec;
> >>
> >> -     size = PAGE_ALIGN(vring_size(num, VIRTIO_PCI_VRING_ALIGN));
> >> -     info->queue = alloc_pages_exact(size,
GFP_KERNEL|__GFP_ZERO);
> >> +     size = vring_size(num, VIRTIO_PCI_VRING_ALIGN);
> >> +     info->queue = dma_zalloc_coherent(vdev->dev.parent,
size,
> >> +                                      
&info->queue_dma_addr, GFP_KERNEL);
> >>       if (info->queue == NULL) {
> >>               err = -ENOMEM;
> >>               goto out_info;
> >>       }
> >>
> >>       /* activate the queue */
> >> -     iowrite32(virt_to_phys(info->queue) >>
VIRTIO_PCI_QUEUE_ADDR_SHIFT,
> >> +     iowrite32(info->queue_dma_addr >>
VIRTIO_PCI_QUEUE_ADDR_SHIFT,
> >>                 vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
> >>
> >>       /* create the vring */
> >> @@ -462,7 +464,8 @@ out_assign:
> >>       vring_del_virtqueue(vq);
> >>  out_activate_queue:
> >>       iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
> >> -     free_pages_exact(info->queue, size);
> >> +     dma_free_coherent(vdev->dev.parent, size,
> >> +                       info->queue, info->queue_dma_addr);
> >>  out_info:
> >>       kfree(info);
> >>       return ERR_PTR(err);
> >> @@ -493,7 +496,8 @@ static void vp_del_vq(struct virtqueue *vq)
> >>       iowrite32(0, vp_dev->ioaddr + VIRTIO_PCI_QUEUE_PFN);
> >>
> >>       size = PAGE_ALIGN(vring_size(info->num,
VIRTIO_PCI_VRING_ALIGN));
> >> -     free_pages_exact(info->queue, size);
> >> +     dma_free_coherent(vq->vdev->dev.parent, size,
> >> +                       info->queue, info->queue_dma_addr);
> >>       kfree(info);
> >>  }
> >>
> >> @@ -712,6 +716,13 @@ static int virtio_pci_probe(struct pci_dev
*pci_dev,
> >>       if (err)
> >>               goto out;
> >>
> >> +     /*
> >> +      * We support 64-bit DMA.  If this fails (e.g. some bridge
> >> +      * or PV code doesn't or DAC is disabled), then
we're okay
> >> +      * with 32-bit DMA.
> >
> > <scratches his head>
> >
> > I am having a hard time parsing that. Could you expand a bit the
> > faulting use-case please?
> >
> >> +      */
> >> +     dma_set_mask_and_coherent(&pci_dev->dev,
DMA_BIT_MASK(64));
> >
> > The usual process is:
> >
> >         ret = dma_set_mask_and_coherent(..)
> >         if (ret)
> >                 ret = dma_set_mask_and_coherent(.., DMA_BIT_MASK(32))
> >
> >         if (ret)
> >                 pr_warn("We are truly screwed. Good
luck!\n");
> >
> 
> I assumed that, if dma_set_mask_and_coherent(..., DMA_BIT_BASK(64))
> fails, then we can still do DMA, just not 64-bit DMA.  This driver
> should be fine with that -- it'll just be a bit slower.
Right. It should fail back to 32-bit (the default on PCI bus).

I was merely thinking of the error reporting might be useful
but on second thought - saying that the user is screwed - is
a bad idea.> 
> If that's not a safe assumption, I can change it.
> 
> --Andy

Possibly Parallel Threads

Search for more reasonably related threads

Linux Virtualization - Aug 2014 - [PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

[PATCH 3/3] virtio_pci: Use the DMA API for virtqueues

Possibly Parallel Threads