* Re: [PATCH] drm/virtio: Use vmalloc for command buffer allocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
@ 2019-08-30 6:08 ` Gerd Hoffmann
[not found] ` <CAASgrz2v0DYb_5A3MnaWFM4Csx1DKkZe546v7DG7R+UyLOA8og@mail.gmail.com>
2019-08-30 16:18 ` Chia-I Wu
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2019-08-30 6:08 UTC (permalink / raw)
To: David Riley
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
Hi,
> {
> if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
> kfree(vbuf->resp_buf);
> - kfree(vbuf->data_buf);
> + kvfree(vbuf->data_buf);
if (is_vmalloc_addr(vbuf->data_buf)) ...
needed here I gues?
> +/* Create sg_table from a vmalloc'd buffer. */
> +static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size)
Hmm, isn't there an existing function for that?
I'd be surprised if virtio-gpu is the first driver needing this ...
And it case there really isn't one this should probably added to the
vmalloc or scatterlist code, not the virtio-gpu driver.
cheers,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH] drm/virtio: Use vmalloc for command buffer allocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
2019-08-30 6:08 ` Gerd Hoffmann
@ 2019-08-30 16:18 ` Chia-I Wu
2019-09-05 22:00 ` [PATCH v2] " David Riley
` (5 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: Chia-I Wu @ 2019-08-30 16:18 UTC (permalink / raw)
To: David Riley
Cc: ML dri-devel, open list:VIRTIO CORE, NET AND BLOCK DRIVERS,
David Airlie, open list, Gurchetan Singh, Gerd Hoffmann,
Stéphane Marchesin
On Thu, Aug 29, 2019 at 2:24 PM David Riley <davidriley@chromium.org> wrote:
>
> Userspace requested command buffer allocations could be too large
> to make as a contiguous allocation. Use vmalloc if necessary to
> satisfy those allocations.
>
> Signed-off-by: David Riley <davidriley@chromium.org>
> ---
> drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 74 ++++++++++++++++++++++++--
> 2 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> index ac60be9b5c19..a8732a8af766 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
> @@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> if (ret)
> goto out_free;
>
> - buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
> + buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
> if (IS_ERR(buf)) {
> ret = PTR_ERR(buf);
> goto out_unresv;
> @@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
> return 0;
>
> out_memdup:
> - kfree(buf);
> + kvfree(buf);
> out_unresv:
> ttm_eu_backoff_reservation(&ticket, &validate_list);
> out_free:
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 981ee16e3ee9..bcbc48b7284f 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
> {
> if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
> kfree(vbuf->resp_buf);
> - kfree(vbuf->data_buf);
> + kvfree(vbuf->data_buf);
> kmem_cache_free(vgdev->vbufs, vbuf);
> }
>
> @@ -251,6 +251,59 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
> wake_up(&vgdev->cursorq.ack_queue);
> }
>
> +/* How many bytes left in this page. */
> +static unsigned int rest_of_page(void *data)
> +{
> + return PAGE_SIZE - offset_in_page(data);
> +}
> +
> +/* Create sg_table from a vmalloc'd buffer. */
> +static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size)
> +{
> + int nents, ret, s, i;
> + struct sg_table *sgt;
> + struct scatterlist *sg;
> + struct page *pg;
> +
> + sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
> + if (!sgt)
> + return NULL;
> +
> + nents = DIV_ROUND_UP(size, PAGE_SIZE) + 1;
> + ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
> + if (ret) {
> + kfree(sgt);
> + return NULL;
> + }
> +
> + for_each_sg(sgt->sgl, sg, nents, i) {
> + pg = vmalloc_to_page(data);
> + if (!pg) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + return NULL;
> + }
> +
> + s = rest_of_page(data);
> + if (s > size)
> + s = size;
> +
> + sg_set_page(sg, pg, s, offset_in_page(data));
> +
> + size -= s;
> + data += s;
> +
> + if (size) {
> + sg_unmark_end(sg);
> + } else {
> + sg_mark_end(sg);
> + break;
> + }
> + }
> +
> + return sgt;
> +}
> +
> static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf)
> __releases(&vgdev->ctrlq.qlock)
> @@ -260,6 +313,7 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> struct scatterlist *sgs[3], vcmd, vout, vresp;
> int outcnt = 0, incnt = 0;
> int ret;
> + struct sg_table *sgt = NULL;
>
> if (!vgdev->vqs_ready)
> return -ENODEV;
> @@ -269,8 +323,17 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> outcnt++;
>
> if (vbuf->data_size) {
> - sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
> - sgs[outcnt + incnt] = &vout;
> + if (is_vmalloc_addr(vbuf->data_buf)) {
> + spin_unlock(&vgdev->ctrlq.qlock);
> + sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size);
> + spin_lock(&vgdev->ctrlq.qlock);
> + if (!sgt)
> + return -ENOMEM;
> + sgs[outcnt + incnt] = sgt->sgl;
If the construction of sgs is no longer atomic, it should be moved out
of the critical section.
> + } else {
> + sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
> + sgs[outcnt + incnt] = &vout;
> + }
> outcnt++;
> }
>
> @@ -294,6 +357,11 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
> virtqueue_kick(vq);
> }
>
> + if (sgt) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + }
> +
> if (!ret)
> ret = vq->num_free;
> return ret;
> --
> 2.23.0.187.g17f5b7556c-goog
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2] drm/virtio: Use vmalloc for command buffer allocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
2019-08-30 6:08 ` Gerd Hoffmann
2019-08-30 16:18 ` Chia-I Wu
@ 2019-09-05 22:00 ` David Riley
2019-09-06 5:18 ` Gerd Hoffmann
2019-09-10 20:06 ` [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Riley @ 2019-09-05 22:00 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley <davidriley@chromium.org>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 114 ++++++++++++++++++++-----
2 files changed, 96 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index ac60be9b5c19..a8732a8af766 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_free;
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
ttm_eu_backoff_reservation(&ticket, &validate_list);
out_free:
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 981ee16e3ee9..3ec89ae8478c 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -251,13 +251,70 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
wake_up(&vgdev->cursorq.ack_queue);
}
+/* How many bytes left in this page. */
+static unsigned int rest_of_page(void *data)
+{
+ return PAGE_SIZE - offset_in_page(data);
+}
+
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int nents, ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ *sg_ents = 0;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ nents = DIV_ROUND_UP(size, PAGE_SIZE) + 1;
+ ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, nents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = rest_of_page(data);
+ if (s > size)
+ s = size;
+
+ sg_set_page(sg, pg, s, offset_in_page(data));
+
+ size -= s;
+ data += s;
+ *sg_ents += 1;
+
+ if (size) {
+ sg_unmark_end(sg);
+ } else {
+ sg_mark_end(sg);
+ break;
+ }
+ }
+
+ return sgt;
+}
+
static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(&vgdev->ctrlq.qlock)
__acquires(&vgdev->ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
int ret;
@@ -268,9 +325,8 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
sgs[outcnt + incnt] = &vcmd;
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] = &vout;
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -299,24 +355,30 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
return ret;
}
-static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
-{
- int rc;
-
- spin_lock(&vgdev->ctrlq.qlock);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(&vgdev->ctrlq.qlock);
- return rc;
-}
-
static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
int rc;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+ &outcnt);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
+ vout = &sg;
+ outcnt = 1;
+ }
+ }
again:
spin_lock(&vgdev->ctrlq.qlock);
@@ -329,19 +391,31 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
* to wait for free space, which can result in fence ids being
* submitted out-of-order.
*/
- if (vq->num_free < 3) {
+ if (vq->num_free < 2 + outcnt) {
spin_unlock(&vgdev->ctrlq.qlock);
wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= 3);
goto again;
}
- if (fence)
+ if (hdr && fence)
virtio_gpu_fence_emit(vgdev, hdr, fence);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
+ rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf, vout);
spin_unlock(&vgdev->ctrlq.qlock);
+
+ if (sgt) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ }
+
return rc;
}
+static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.187.g17f5b7556c-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/virtio: Use vmalloc for command buffer allocations.
2019-09-05 22:00 ` [PATCH v2] " David Riley
@ 2019-09-06 5:18 ` Gerd Hoffmann
2019-09-09 17:12 ` David Riley
0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2019-09-06 5:18 UTC (permalink / raw)
To: David Riley
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
> +/* How many bytes left in this page. */
> +static unsigned int rest_of_page(void *data)
> +{
> + return PAGE_SIZE - offset_in_page(data);
> +}
Not needed.
> +/* Create sg_table from a vmalloc'd buffer. */
> +static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
> +{
> + int nents, ret, s, i;
> + struct sg_table *sgt;
> + struct scatterlist *sg;
> + struct page *pg;
> +
> + *sg_ents = 0;
> +
> + sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
> + if (!sgt)
> + return NULL;
> +
> + nents = DIV_ROUND_UP(size, PAGE_SIZE) + 1;
Why +1?
> + ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
> + if (ret) {
> + kfree(sgt);
> + return NULL;
> + }
> +
> + for_each_sg(sgt->sgl, sg, nents, i) {
> + pg = vmalloc_to_page(data);
> + if (!pg) {
> + sg_free_table(sgt);
> + kfree(sgt);
> + return NULL;
> + }
> +
> + s = rest_of_page(data);
> + if (s > size)
> + s = size;
vmalloc memory is page aligned, so:
s = min(PAGE_SIZE, size);
> + sg_set_page(sg, pg, s, offset_in_page(data));
Offset is always zero.
> +
> + size -= s;
> + data += s;
> + *sg_ents += 1;
sg_ents isn't used anywhere.
> +
> + if (size) {
> + sg_unmark_end(sg);
> + } else {
> + sg_mark_end(sg);
> + break;
> + }
That looks a bit strange. I guess you need only one of the two because
the other is the default?
> static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
> struct virtio_gpu_vbuffer *vbuf,
> struct virtio_gpu_ctrl_hdr *hdr,
> struct virtio_gpu_fence *fence)
> {
> struct virtqueue *vq = vgdev->ctrlq.vq;
> + struct scatterlist *vout = NULL, sg;
> + struct sg_table *sgt = NULL;
> int rc;
> + int outcnt = 0;
> +
> + if (vbuf->data_size) {
> + if (is_vmalloc_addr(vbuf->data_buf)) {
> + sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
> + &outcnt);
> + if (!sgt)
> + return -ENOMEM;
> + vout = sgt->sgl;
> + } else {
> + sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
> + vout = &sg;
> + outcnt = 1;
outcnt must be set in both cases.
> +static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
> + struct virtio_gpu_vbuffer *vbuf)
> +{
> + return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
> +}
Changing virtio_gpu_queue_ctrl_buffer to call
virtio_gpu_queue_fenced_ctrl_buffer should be done in a separate patch.
cheers,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2] drm/virtio: Use vmalloc for command buffer allocations.
2019-09-06 5:18 ` Gerd Hoffmann
@ 2019-09-09 17:12 ` David Riley
0 siblings, 0 replies; 20+ messages in thread
From: David Riley @ 2019-09-09 17:12 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
On Thu, Sep 5, 2019 at 10:18 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> > +/* How many bytes left in this page. */
> > +static unsigned int rest_of_page(void *data)
> > +{
> > + return PAGE_SIZE - offset_in_page(data);
> > +}
>
> Not needed.
>
> > +/* Create sg_table from a vmalloc'd buffer. */
> > +static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
> > +{
> > + int nents, ret, s, i;
> > + struct sg_table *sgt;
> > + struct scatterlist *sg;
> > + struct page *pg;
> > +
> > + *sg_ents = 0;
> > +
> > + sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
> > + if (!sgt)
> > + return NULL;
> > +
> > + nents = DIV_ROUND_UP(size, PAGE_SIZE) + 1;
>
> Why +1?
This is part of handling offsets within the vmalloc buffer and to
maintain parity with the !is_vmalloc_addr/existing case (sg_init_one
handles offsets within pages internally). I had left it in because
this is being used for all sg/descriptor generation and I wasn't sure
if someone in the future might do something like:
buf = vmemdup_user()
offset = find_interesting(buf)
queue(buf + offset)
To respond specifically to your question, if we handle offsets, a
vmalloc_to_sgt(size = PAGE_SIZE + 2) could end up with 3 sg_ents with
the +1 being to account for that extra page.
I'll just remove all support for offsets in v3 of the patch and
comment that functionality will be different based on where the buffer
was originally allocated from.
>
> > + ret = sg_alloc_table(sgt, nents, GFP_KERNEL);
> > + if (ret) {
> > + kfree(sgt);
> > + return NULL;
> > + }
> > +
> > + for_each_sg(sgt->sgl, sg, nents, i) {
> > + pg = vmalloc_to_page(data);
> > + if (!pg) {
> > + sg_free_table(sgt);
> > + kfree(sgt);
> > + return NULL;
> > + }
> > +
> > + s = rest_of_page(data);
> > + if (s > size)
> > + s = size;
>
> vmalloc memory is page aligned, so:
As per above, will remove with v3.
>
> s = min(PAGE_SIZE, size);
>
> > + sg_set_page(sg, pg, s, offset_in_page(data));
>
> Offset is always zero.
As per above, will remove with v3.
>
> > +
> > + size -= s;
> > + data += s;
> > + *sg_ents += 1;
>
> sg_ents isn't used anywhere.
It's used for outcnt.
>
> > +
> > + if (size) {
> > + sg_unmark_end(sg);
> > + } else {
> > + sg_mark_end(sg);
> > + break;
> > + }
>
> That looks a bit strange. I guess you need only one of the two because
> the other is the default?
I was being overly paranoid and not wanting to make assumptions about
the initial state of the table. I'll simplify.
>
> > static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
> > struct virtio_gpu_vbuffer *vbuf,
> > struct virtio_gpu_ctrl_hdr *hdr,
> > struct virtio_gpu_fence *fence)
> > {
> > struct virtqueue *vq = vgdev->ctrlq.vq;
> > + struct scatterlist *vout = NULL, sg;
> > + struct sg_table *sgt = NULL;
> > int rc;
> > + int outcnt = 0;
> > +
> > + if (vbuf->data_size) {
> > + if (is_vmalloc_addr(vbuf->data_buf)) {
> > + sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
> > + &outcnt);
> > + if (!sgt)
> > + return -ENOMEM;
> > + vout = sgt->sgl;
> > + } else {
> > + sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
> > + vout = &sg;
> > + outcnt = 1;
>
> outcnt must be set in both cases.
outcnt is set by vmalloc_to_sgt.
>
> > +static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
> > + struct virtio_gpu_vbuffer *vbuf)
> > +{
> > + return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
> > +}
>
> Changing virtio_gpu_queue_ctrl_buffer to call
> virtio_gpu_queue_fenced_ctrl_buffer should be done in a separate patch.
Will do.
Thanks,
David
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (2 preceding siblings ...)
2019-09-05 22:00 ` [PATCH v2] " David Riley
@ 2019-09-10 20:06 ` David Riley
2019-09-11 5:12 ` Gerd Hoffmann
2019-09-10 20:06 ` [PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: David Riley @ 2019-09-10 20:06 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Factor function in preparation to generating scatterlist prior to locking.
Signed-off-by: David Riley <davidriley@chromium.org>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 981ee16e3ee9..bf5a4a50b002 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -299,17 +299,6 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
return ret;
}
-static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
-{
- int rc;
-
- spin_lock(&vgdev->ctrlq.qlock);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(&vgdev->ctrlq.qlock);
- return rc;
-}
-
static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
@@ -335,13 +324,19 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
goto again;
}
- if (fence)
+ if (hdr && fence)
virtio_gpu_fence_emit(vgdev, hdr, fence);
rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
spin_unlock(&vgdev->ctrlq.qlock);
return rc;
}
+static int virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ return virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static int virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
2019-09-10 20:06 ` [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
@ 2019-09-11 5:12 ` Gerd Hoffmann
2019-09-11 17:35 ` David Riley
0 siblings, 1 reply; 20+ messages in thread
From: Gerd Hoffmann @ 2019-09-11 5:12 UTC (permalink / raw)
To: David Riley
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
On Tue, Sep 10, 2019 at 01:06:50PM -0700, David Riley wrote:
> Factor function in preparation to generating scatterlist prior to locking.
Patches are looking good now, but they don't apply. What tree was used
to create them?
Latest virtio-gpu driver bits are in drm-misc-next (see
https://cgit.freedesktop.org/drm/drm-misc).
cheers,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
2019-09-11 5:12 ` Gerd Hoffmann
@ 2019-09-11 17:35 ` David Riley
0 siblings, 0 replies; 20+ messages in thread
From: David Riley @ 2019-09-11 17:35 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
They were based off of Linus' https://github.com/torvalds/linux
master from yesterday.
I can rebase onto drm-misc-next.
On Tue, Sep 10, 2019 at 10:12 PM Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> On Tue, Sep 10, 2019 at 01:06:50PM -0700, David Riley wrote:
> > Factor function in preparation to generating scatterlist prior to locking.
>
> Patches are looking good now, but they don't apply. What tree was used
> to create them?
>
> Latest virtio-gpu driver bits are in drm-misc-next (see
> https://cgit.freedesktop.org/drm/drm-misc).
>
> cheers,
> Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (3 preceding siblings ...)
2019-09-10 20:06 ` [PATCH v3 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
@ 2019-09-10 20:06 ` David Riley
2019-09-11 18:14 ` [PATCH v4 0/2] drm/virtio: Use vmalloc for command buffer alllocations David Riley
` (2 subsequent siblings)
7 siblings, 0 replies; 20+ messages in thread
From: David Riley @ 2019-09-10 20:06 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley <davidriley@chromium.org>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 79 +++++++++++++++++++++++---
2 files changed, 73 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index ac60be9b5c19..a8732a8af766 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -195,7 +195,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
if (ret)
goto out_free;
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -230,7 +230,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
ttm_eu_backoff_reservation(&ticket, &validate_list);
out_free:
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index bf5a4a50b002..76cf2b9d5d1d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -154,7 +154,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -251,13 +251,54 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
wake_up(&vgdev->cursorq.ack_queue);
}
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ if (WARN_ON(!PAGE_ALIGNED(data)))
+ return NULL;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ *sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
+ ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = min_t(int, PAGE_SIZE, size);
+ sg_set_page(sg, pg, s, 0);
+
+ size -= s;
+ data += s;
+ }
+
+ return sgt;
+}
+
static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(&vgdev->ctrlq.qlock)
__acquires(&vgdev->ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
int ret;
@@ -268,9 +309,8 @@ static int virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
sgs[outcnt + incnt] = &vcmd;
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] = &vout;
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -305,7 +345,24 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
int rc;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+ &outcnt);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
+ vout = &sg;
+ outcnt = 1;
+ }
+ }
again:
spin_lock(&vgdev->ctrlq.qlock);
@@ -318,7 +375,7 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
* to wait for free space, which can result in fence ids being
* submitted out-of-order.
*/
- if (vq->num_free < 3) {
+ if (vq->num_free < 2 + outcnt) {
spin_unlock(&vgdev->ctrlq.qlock);
wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= 3);
goto again;
@@ -326,8 +383,14 @@ static int virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
if (hdr && fence)
virtio_gpu_fence_emit(vgdev, hdr, fence);
- rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
+ rc = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf, vout);
spin_unlock(&vgdev->ctrlq.qlock);
+
+ if (sgt) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ }
+
return rc;
}
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 0/2] drm/virtio: Use vmalloc for command buffer alllocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (4 preceding siblings ...)
2019-09-10 20:06 ` [PATCH v3 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
@ 2019-09-11 18:14 ` David Riley
2019-09-11 18:14 ` [PATCH v4 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
2019-09-11 18:14 ` [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
7 siblings, 0 replies; 20+ messages in thread
From: David Riley @ 2019-09-11 18:14 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
v1: Initial version.
v2: Properly account for number of free descriptors required.
v3: Remove offset handling for vmalloc'd buffers.
v4: Rebase onto drm-misc-next.
David Riley (2):
drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
drm/virtio: Use vmalloc for command buffer allocations.
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 98 ++++++++++++++++++++------
2 files changed, 79 insertions(+), 23 deletions(-)
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v4 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (5 preceding siblings ...)
2019-09-11 18:14 ` [PATCH v4 0/2] drm/virtio: Use vmalloc for command buffer alllocations David Riley
@ 2019-09-11 18:14 ` David Riley
2019-09-11 18:14 ` [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
7 siblings, 0 replies; 20+ messages in thread
From: David Riley @ 2019-09-11 18:14 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Factor function in preparation to generating scatterlist prior to locking.
Signed-off-by: David Riley <davidriley@chromium.org>
---
drivers/gpu/drm/virtio/virtgpu_vq.c | 20 +++++++-------------
1 file changed, 7 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 7fd2851f7b97..5a64c776138d 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -302,18 +302,6 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
return notify;
}
-static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
-{
- bool notify;
-
- spin_lock(&vgdev->ctrlq.qlock);
- notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
- spin_unlock(&vgdev->ctrlq.qlock);
- if (notify)
- virtqueue_notify(vgdev->ctrlq.vq);
-}
-
static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf,
struct virtio_gpu_ctrl_hdr *hdr,
@@ -339,7 +327,7 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
goto again;
}
- if (fence) {
+ if (hdr && fence) {
virtio_gpu_fence_emit(vgdev, hdr, fence);
if (vbuf->objs) {
virtio_gpu_array_add_fence(vbuf->objs, &fence->f);
@@ -352,6 +340,12 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
virtqueue_notify(vgdev->ctrlq.vq);
}
+static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
+ struct virtio_gpu_vbuffer *vbuf)
+{
+ virtio_gpu_queue_fenced_ctrl_buffer(vgdev, vbuf, NULL, NULL);
+}
+
static void virtio_gpu_queue_cursor(struct virtio_gpu_device *vgdev,
struct virtio_gpu_vbuffer *vbuf)
{
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations.
2019-08-29 21:24 [PATCH] drm/virtio: Use vmalloc for command buffer allocations David Riley
` (6 preceding siblings ...)
2019-09-11 18:14 ` [PATCH v4 1/2] drm/virtio: Rewrite virtio_gpu_queue_ctrl_buffer using fenced version David Riley
@ 2019-09-11 18:14 ` David Riley
2019-09-12 7:51 ` Gerd Hoffmann
7 siblings, 1 reply; 20+ messages in thread
From: David Riley @ 2019-09-11 18:14 UTC (permalink / raw)
To: dri-devel, virtualization
Cc: David Airlie, Gerd Hoffmann, Daniel Vetter, Gurchetan Singh,
Stéphane Marchesin, linux-kernel, David Riley
Userspace requested command buffer allocations could be too large
to make as a contiguous allocation. Use vmalloc if necessary to
satisfy those allocations.
Signed-off-by: David Riley <davidriley@chromium.org>
---
drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 78 +++++++++++++++++++++++---
2 files changed, 72 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
index f5083c538f9c..9af1ec62434f 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
@@ -143,7 +143,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
goto out_unused_fd;
}
- buf = memdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
+ buf = vmemdup_user(u64_to_user_ptr(exbuf->command), exbuf->size);
if (IS_ERR(buf)) {
ret = PTR_ERR(buf);
goto out_unresv;
@@ -172,7 +172,7 @@ static int virtio_gpu_execbuffer_ioctl(struct drm_device *dev, void *data,
return 0;
out_memdup:
- kfree(buf);
+ kvfree(buf);
out_unresv:
if (buflist)
virtio_gpu_array_unlock_resv(buflist);
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a64c776138d..9f9b782dd332 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -155,7 +155,7 @@ static void free_vbuf(struct virtio_gpu_device *vgdev,
{
if (vbuf->resp_size > MAX_INLINE_RESP_SIZE)
kfree(vbuf->resp_buf);
- kfree(vbuf->data_buf);
+ kvfree(vbuf->data_buf);
kmem_cache_free(vgdev->vbufs, vbuf);
}
@@ -256,13 +256,54 @@ void virtio_gpu_dequeue_cursor_func(struct work_struct *work)
wake_up(&vgdev->cursorq.ack_queue);
}
+/* Create sg_table from a vmalloc'd buffer. */
+static struct sg_table *vmalloc_to_sgt(char *data, uint32_t size, int *sg_ents)
+{
+ int ret, s, i;
+ struct sg_table *sgt;
+ struct scatterlist *sg;
+ struct page *pg;
+
+ if (WARN_ON(!PAGE_ALIGNED(data)))
+ return NULL;
+
+ sgt = kmalloc(sizeof(*sgt), GFP_KERNEL);
+ if (!sgt)
+ return NULL;
+
+ *sg_ents = DIV_ROUND_UP(size, PAGE_SIZE);
+ ret = sg_alloc_table(sgt, *sg_ents, GFP_KERNEL);
+ if (ret) {
+ kfree(sgt);
+ return NULL;
+ }
+
+ for_each_sg(sgt->sgl, sg, *sg_ents, i) {
+ pg = vmalloc_to_page(data);
+ if (!pg) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ return NULL;
+ }
+
+ s = min_t(int, PAGE_SIZE, size);
+ sg_set_page(sg, pg, s, 0);
+
+ size -= s;
+ data += s;
+ }
+
+ return sgt;
+}
+
static bool virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
- struct virtio_gpu_vbuffer *vbuf)
+ struct virtio_gpu_vbuffer *vbuf,
+ struct scatterlist *vout)
__releases(&vgdev->ctrlq.qlock)
__acquires(&vgdev->ctrlq.qlock)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
- struct scatterlist *sgs[3], vcmd, vout, vresp;
+ struct scatterlist *sgs[3], vcmd, vresp;
int outcnt = 0, incnt = 0;
bool notify = false;
int ret;
@@ -274,9 +315,8 @@ static bool virtio_gpu_queue_ctrl_buffer_locked(struct virtio_gpu_device *vgdev,
sgs[outcnt + incnt] = &vcmd;
outcnt++;
- if (vbuf->data_size) {
- sg_init_one(&vout, vbuf->data_buf, vbuf->data_size);
- sgs[outcnt + incnt] = &vout;
+ if (vout) {
+ sgs[outcnt + incnt] = vout;
outcnt++;
}
@@ -308,7 +348,24 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
struct virtio_gpu_fence *fence)
{
struct virtqueue *vq = vgdev->ctrlq.vq;
+ struct scatterlist *vout = NULL, sg;
+ struct sg_table *sgt = NULL;
bool notify;
+ int outcnt = 0;
+
+ if (vbuf->data_size) {
+ if (is_vmalloc_addr(vbuf->data_buf)) {
+ sgt = vmalloc_to_sgt(vbuf->data_buf, vbuf->data_size,
+ &outcnt);
+ if (!sgt)
+ return -ENOMEM;
+ vout = sgt->sgl;
+ } else {
+ sg_init_one(&sg, vbuf->data_buf, vbuf->data_size);
+ vout = &sg;
+ outcnt = 1;
+ }
+ }
again:
spin_lock(&vgdev->ctrlq.qlock);
@@ -321,7 +378,7 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
* to wait for free space, which can result in fence ids being
* submitted out-of-order.
*/
- if (vq->num_free < 3) {
+ if (vq->num_free < 2 + outcnt) {
spin_unlock(&vgdev->ctrlq.qlock);
wait_event(vgdev->ctrlq.ack_queue, vq->num_free >= 3);
goto again;
@@ -334,10 +391,15 @@ static void virtio_gpu_queue_fenced_ctrl_buffer(struct virtio_gpu_device *vgdev,
virtio_gpu_array_unlock_resv(vbuf->objs);
}
}
- notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf);
+ notify = virtio_gpu_queue_ctrl_buffer_locked(vgdev, vbuf, vout);
spin_unlock(&vgdev->ctrlq.qlock);
if (notify)
virtqueue_notify(vgdev->ctrlq.vq);
+
+ if (sgt) {
+ sg_free_table(sgt);
+ kfree(sgt);
+ }
}
static void virtio_gpu_queue_ctrl_buffer(struct virtio_gpu_device *vgdev,
--
2.23.0.162.g0b9fbb3734-goog
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations.
2019-09-11 18:14 ` [PATCH v4 2/2] drm/virtio: Use vmalloc for command buffer allocations David Riley
@ 2019-09-12 7:51 ` Gerd Hoffmann
0 siblings, 0 replies; 20+ messages in thread
From: Gerd Hoffmann @ 2019-09-12 7:51 UTC (permalink / raw)
To: David Riley
Cc: dri-devel, virtualization, David Airlie, Daniel Vetter,
Gurchetan Singh, Stéphane Marchesin, linux-kernel
On Wed, Sep 11, 2019 at 11:14:03AM -0700, David Riley wrote:
> Userspace requested command buffer allocations could be too large
> to make as a contiguous allocation. Use vmalloc if necessary to
> satisfy those allocations.
Both applied to drm-misc-next.
thanks,
Gerd
^ permalink raw reply [flat|nested] 20+ messages in thread