On Tue, Feb 2, 2016 at 3:25 AM, Michael S. Tsirkin <mst@xxxxxxxxxx> wrote:
> On Mon, Feb 01, 2016 at 10:00:56AM -0800, Andy Lutomirski wrote:
>> This leaves vring_new_virtqueue alone for compatbility, but it
>> adds two new improved APIs:
>>
>> vring_create_virtqueue: Creates a virtqueue backed by automatically
>> allocated coherent memory. (Some day it this could be extended to
>> support non-coherent memory, too, if there ends up being a platform
>> on which it's worthwhile.)
>>
>> __vring_new_virtqueue: Creates a virtqueue with a manually-specified
>> layout. This should allow mic_virtio to work much more cleanly.
>>
>> Signed-off-by: Andy Lutomirski <luto@xxxxxxxxxx>
>> ---
>> drivers/virtio/virtio_ring.c | 178 +++++++++++++++++++++++++++++++++++--------
>> include/linux/virtio.h | 23 +++++-
>> include/linux/virtio_ring.h | 35 +++++++++
>> 3 files changed, 204 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
>> index 2f621e96b9ff..cf2840c7e500 100644
>> --- a/drivers/virtio/virtio_ring.c
>> +++ b/drivers/virtio/virtio_ring.c
>> @@ -95,6 +95,11 @@ struct vring_virtqueue {
>> /* How to notify other side. FIXME: commonalize hcalls! */
>> bool (*notify)(struct virtqueue *vq);
>>
>> + /* DMA, allocation, and size information */
>> + bool we_own_ring;
>> + size_t queue_size_in_bytes;
>> + dma_addr_t queue_dma_addr;
>> +
>> #ifdef DEBUG
>> /* They're supposed to lock for us. */
>> unsigned int in_use;
>> @@ -878,36 +883,31 @@ irqreturn_t vring_interrupt(int irq, void *_vq)
>> }
>> EXPORT_SYMBOL_GPL(vring_interrupt);
>>
>> -struct virtqueue *vring_new_virtqueue(unsigned int index,
>> - unsigned int num,
>> - unsigned int vring_align,
>> - struct virtio_device *vdev,
>> - bool weak_barriers,
>> - void *pages,
>> - bool (*notify)(struct virtqueue *),
>> - void (*callback)(struct virtqueue *),
>> - const char *name)
>> +struct virtqueue *__vring_new_virtqueue(unsigned int index,
>> + struct vring vring,
>> + struct virtio_device *vdev,
>> + bool weak_barriers,
>> + bool (*notify)(struct virtqueue *),
>> + void (*callback)(struct virtqueue *),
>> + const char *name)
>> {
>> - struct vring_virtqueue *vq;
>> unsigned int i;
>> + struct vring_virtqueue *vq;
>>
>> - /* We assume num is a power of 2. */
>> - if (num & (num - 1)) {
>> - dev_warn(&vdev->dev, "Bad virtqueue length %u\n", num);
>> - return NULL;
>> - }
>> -
>> - vq = kmalloc(sizeof(*vq) + num * sizeof(struct vring_desc_state),
>> + vq = kmalloc(sizeof(*vq) + vring.num * sizeof(struct vring_desc_state),
>> GFP_KERNEL);
>> if (!vq)
>> return NULL;
>>
>> - vring_init(&vq->vring, num, pages, vring_align);
>> + vq->vring = vring;
>> vq->vq.callback = callback;
>> vq->vq.vdev = vdev;
>> vq->vq.name = name;
>> - vq->vq.num_free = num;
>> + vq->vq.num_free = vring.num;
>> vq->vq.index = index;
>> + vq->we_own_ring = false;
>> + vq->queue_dma_addr = 0;
>> + vq->queue_size_in_bytes = 0;
>> vq->notify = notify;
>> vq->weak_barriers = weak_barriers;
>> vq->broken = false;
>> @@ -932,18 +932,105 @@ struct virtqueue *vring_new_virtqueue(unsigned int index,
>>
>> /* Put everything in free lists. */
>> vq->free_head = 0;
>> - for (i = 0; i < num-1; i++)
>> + for (i = 0; i < vring.num-1; i++)
>> vq->vring.desc[i].next = cpu_to_virtio16(vdev, i + 1);
>> - memset(vq->desc_state, 0, num * sizeof(struct vring_desc_state));
>> + memset(vq->desc_state, 0, vring.num * sizeof(struct vring_desc_state));
>>
>> return &vq->vq;
>> }
>> +EXPORT_SYMBOL_GPL(__vring_new_virtqueue);
>> +
>> +struct virtqueue *vring_create_virtqueue(
>> + unsigned int index,
>> + unsigned int num,
>> + unsigned int vring_align,
>> + struct virtio_device *vdev,
>> + bool weak_barriers,
>> + bool may_reduce_num,
>> + bool (*notify)(struct virtqueue *),
>> + void (*callback)(struct virtqueue *),
>> + const char *name)
>> +{
>> + struct virtqueue *vq;
>> + void *queue;
>> + dma_addr_t dma_addr;
>> + size_t queue_size_in_bytes;
>> + struct vring vring;
>> +
>> + /* We assume num is a power of 2. */
>> + if (num & (num - 1)) {
>> + dev_warn(&vdev->dev, "Bad virtqueue length %u\n", num);
>> + return NULL;
>> + }
>> +
>> + /* TODO: allocate each queue chunk individually */
>> + for (; num && vring_size(num, vring_align) > PAGE_SIZE; num /= 2) {
>> + queue = dma_zalloc_coherent(
>> + vdev->dev.parent, vring_size(num, vring_align),
>> + &dma_addr, GFP_KERNEL|__GFP_NOWARN);
>
> I think that we should teach this one to use regular kmalloc
> if vring_use_dma_api is cleared.
> Not a must but it seems cleaner at this stage.
Done. It arguably makes the code simpler, too, since I can just set
dma_addr to virt_to_phys(queue).
--Andy
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
