* [PATCH] virtio_console: Stop doing DMA on the stack
@ 2016-08-30 15:04 Andy Lutomirski
2016-09-06 7:03 ` Amit Shah
0 siblings, 1 reply; 5+ messages in thread
From: Andy Lutomirski @ 2016-08-30 15:04 UTC (permalink / raw)
To: x86, Amit Shah, virtualization
Cc: linux-kernel, Andy Lutomirski, Michael S. Tsirkin
virtio_console uses a small DMA buffer for control requests. Move
that buffer into heap memory.
Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
systems (which is currently most of them), but it breaks completely
if the stack is virtually mapped.
Tested by typing both directions using picocom aimed at /dev/hvc0.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
Hi all-
This is currently broken in tip:x86/asm. If you (Amit) like this patch,
would it make sense for Ingo to add it to -tip?
Thanks,
Andy
drivers/char/virtio_console.c | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
index d2406fe25533..5da47e26a012 100644
--- a/drivers/char/virtio_console.c
+++ b/drivers/char/virtio_console.c
@@ -165,6 +165,12 @@ struct ports_device {
*/
struct virtqueue *c_ivq, *c_ovq;
+ /*
+ * A control packet buffer for guest->host requests, protected
+ * by c_ovq_lock.
+ */
+ struct virtio_console_control cpkt;
+
/* Array of per-port IO virtqueues */
struct virtqueue **in_vqs, **out_vqs;
@@ -560,28 +566,29 @@ static ssize_t __send_control_msg(struct ports_device *portdev, u32 port_id,
unsigned int event, unsigned int value)
{
struct scatterlist sg[1];
- struct virtio_console_control cpkt;
struct virtqueue *vq;
unsigned int len;
if (!use_multiport(portdev))
return 0;
- cpkt.id = cpu_to_virtio32(portdev->vdev, port_id);
- cpkt.event = cpu_to_virtio16(portdev->vdev, event);
- cpkt.value = cpu_to_virtio16(portdev->vdev, value);
-
vq = portdev->c_ovq;
- sg_init_one(sg, &cpkt, sizeof(cpkt));
-
spin_lock(&portdev->c_ovq_lock);
- if (virtqueue_add_outbuf(vq, sg, 1, &cpkt, GFP_ATOMIC) == 0) {
+
+ portdev->cpkt.id = cpu_to_virtio32(portdev->vdev, port_id);
+ portdev->cpkt.event = cpu_to_virtio16(portdev->vdev, event);
+ portdev->cpkt.value = cpu_to_virtio16(portdev->vdev, value);
+
+ sg_init_one(sg, &portdev->cpkt, sizeof(struct virtio_console_control));
+
+ if (virtqueue_add_outbuf(vq, sg, 1, &portdev->cpkt, GFP_ATOMIC) == 0) {
virtqueue_kick(vq);
while (!virtqueue_get_buf(vq, &len)
&& !virtqueue_is_broken(vq))
cpu_relax();
}
+
spin_unlock(&portdev->c_ovq_lock);
return 0;
}
--
2.7.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_console: Stop doing DMA on the stack
2016-08-30 15:04 [PATCH] virtio_console: Stop doing DMA on the stack Andy Lutomirski
@ 2016-09-06 7:03 ` Amit Shah
2016-09-08 6:49 ` Ingo Molnar
[not found] ` <20160908064943.GA25876@gmail.com>
0 siblings, 2 replies; 5+ messages in thread
From: Amit Shah @ 2016-09-06 7:03 UTC (permalink / raw)
To: Andy Lutomirski; +Cc: x86, Michael S. Tsirkin, linux-kernel, virtualization
On (Tue) 30 Aug 2016 [08:04:15], Andy Lutomirski wrote:
> virtio_console uses a small DMA buffer for control requests. Move
> that buffer into heap memory.
>
> Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
> systems (which is currently most of them), but it breaks completely
> if the stack is virtually mapped.
>
> Tested by typing both directions using picocom aimed at /dev/hvc0.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
Looks fine,
Reviewed-by: Amit Shah <amit.shah@redhat.com>
> ---
>
> Hi all-
>
> This is currently broken in tip:x86/asm. If you (Amit) like this patch,
> would it make sense for Ingo to add it to -tip?
Yes, I'm fine with that.
Amit
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_console: Stop doing DMA on the stack
2016-09-06 7:03 ` Amit Shah
@ 2016-09-08 6:49 ` Ingo Molnar
[not found] ` <20160908064943.GA25876@gmail.com>
1 sibling, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2016-09-08 6:49 UTC (permalink / raw)
To: Amit Shah
Cc: x86, Michael S. Tsirkin, linux-kernel, Andy Lutomirski,
virtualization
* Amit Shah <amit.shah@redhat.com> wrote:
> On (Tue) 30 Aug 2016 [08:04:15], Andy Lutomirski wrote:
> > virtio_console uses a small DMA buffer for control requests. Move
> > that buffer into heap memory.
> >
> > Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
> > systems (which is currently most of them), but it breaks completely
> > if the stack is virtually mapped.
> >
> > Tested by typing both directions using picocom aimed at /dev/hvc0.
> >
> > Signed-off-by: Andy Lutomirski <luto@kernel.org>
>
> Looks fine,
>
> Reviewed-by: Amit Shah <amit.shah@redhat.com>
>
> > ---
> >
> > Hi all-
> >
> > This is currently broken in tip:x86/asm. If you (Amit) like this patch,
> > would it make sense for Ingo to add it to -tip?
>
> Yes, I'm fine with that.
Thanks! FYI, this patch now lives as:
9472fe7040bb ("virtio_console: Stop doing DMA on the stack")
in tip:x86/asm, and is targeted for a v4.9 merge.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
[parent not found: <20160908064943.GA25876@gmail.com>]
* Re: [PATCH] virtio_console: Stop doing DMA on the stack
[not found] ` <20160908064943.GA25876@gmail.com>
@ 2016-09-09 18:10 ` Michael S. Tsirkin
2016-09-10 4:33 ` Ingo Molnar
0 siblings, 1 reply; 5+ messages in thread
From: Michael S. Tsirkin @ 2016-09-09 18:10 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Amit Shah, x86, linux-kernel, Andy Lutomirski, virtualization
On Thu, Sep 08, 2016 at 08:49:43AM +0200, Ingo Molnar wrote:
>
> * Amit Shah <amit.shah@redhat.com> wrote:
>
> > On (Tue) 30 Aug 2016 [08:04:15], Andy Lutomirski wrote:
> > > virtio_console uses a small DMA buffer for control requests. Move
> > > that buffer into heap memory.
> > >
> > > Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
> > > systems (which is currently most of them), but it breaks completely
> > > if the stack is virtually mapped.
> > >
> > > Tested by typing both directions using picocom aimed at /dev/hvc0.
> > >
> > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> >
> > Looks fine,
> >
> > Reviewed-by: Amit Shah <amit.shah@redhat.com>
> >
> > > ---
> > >
> > > Hi all-
> > >
> > > This is currently broken in tip:x86/asm. If you (Amit) like this patch,
> > > would it make sense for Ingo to add it to -tip?
> >
> > Yes, I'm fine with that.
>
> Thanks! FYI, this patch now lives as:
>
> 9472fe7040bb ("virtio_console: Stop doing DMA on the stack")
>
> in tip:x86/asm, and is targeted for a v4.9 merge.
>
> Thanks,
>
> Ingo
Thinking about it, maybe we should put it in 4.8
after all, for benefit of systems using DMA API with virtio.
Thoughts?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] virtio_console: Stop doing DMA on the stack
2016-09-09 18:10 ` Michael S. Tsirkin
@ 2016-09-10 4:33 ` Ingo Molnar
0 siblings, 0 replies; 5+ messages in thread
From: Ingo Molnar @ 2016-09-10 4:33 UTC (permalink / raw)
To: Michael S. Tsirkin
Cc: Amit Shah, x86, linux-kernel, Andy Lutomirski, virtualization
* Michael S. Tsirkin <mst@redhat.com> wrote:
> On Thu, Sep 08, 2016 at 08:49:43AM +0200, Ingo Molnar wrote:
> >
> > * Amit Shah <amit.shah@redhat.com> wrote:
> >
> > > On (Tue) 30 Aug 2016 [08:04:15], Andy Lutomirski wrote:
> > > > virtio_console uses a small DMA buffer for control requests. Move
> > > > that buffer into heap memory.
> > > >
> > > > Doing virtio DMA on the stack is normally okay on non-DMA-API virtio
> > > > systems (which is currently most of them), but it breaks completely
> > > > if the stack is virtually mapped.
> > > >
> > > > Tested by typing both directions using picocom aimed at /dev/hvc0.
> > > >
> > > > Signed-off-by: Andy Lutomirski <luto@kernel.org>
> > >
> > > Looks fine,
> > >
> > > Reviewed-by: Amit Shah <amit.shah@redhat.com>
> > >
> > > > ---
> > > >
> > > > Hi all-
> > > >
> > > > This is currently broken in tip:x86/asm. If you (Amit) like this patch,
> > > > would it make sense for Ingo to add it to -tip?
> > >
> > > Yes, I'm fine with that.
> >
> > Thanks! FYI, this patch now lives as:
> >
> > 9472fe7040bb ("virtio_console: Stop doing DMA on the stack")
> >
> > in tip:x86/asm, and is targeted for a v4.9 merge.
> >
> > Thanks,
> >
> > Ingo
>
> Thinking about it, maybe we should put it in 4.8
> after all, for benefit of systems using DMA API with virtio.
> Thoughts?
So CONFIG_VMAP_STACK=y is only going to be enabled for v4.9 - the enabling commit
is part of tip:x86/asm as well. So AFAICS the commit is not strictly needed for
v4.8.
Thanks,
Ingo
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-09-10 4:33 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-08-30 15:04 [PATCH] virtio_console: Stop doing DMA on the stack Andy Lutomirski
2016-09-06 7:03 ` Amit Shah
2016-09-08 6:49 ` Ingo Molnar
[not found] ` <20160908064943.GA25876@gmail.com>
2016-09-09 18:10 ` Michael S. Tsirkin
2016-09-10 4:33 ` Ingo Molnar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).