Re: [PATCH] virtio_console: Stop doing DMA on the stack

[Ingo Molnar]

* Michael S. Tsirkin  wrote:

> On Thu, Sep 08, 2016 at 08:49:43AM +0200, Ingo Molnar wrote:
> > 
> > * Amit Shah  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 
> > > 
> > > Looks fine,
> > > 
> > > Reviewed-by: Amit Shah 
> > > 
> > > > ---
> > > > 
> > > > 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_console: Stop doing DMA on the stack

[Michael S. Tsirkin]

On Thu, Sep 08, 2016 at 08:49:43AM +0200, Ingo Molnar wrote:
> 
> * Amit Shah  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 
> > 
> > Looks fine,
> > 
> > Reviewed-by: Amit Shah 
> > 
> > > ---
> > > 
> > > 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?
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_console: Stop doing DMA on the stack

[Ingo Molnar]

* Amit Shah  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 
> 
> Looks fine,
> 
> Reviewed-by: Amit Shah 
> 
> > ---
> > 
> > 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: [PATCH] virtio_console: Stop doing DMA on the stack

[Amit Shah]

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 

Looks fine,

Reviewed-by: Amit Shah 

> ---
> 
> 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
___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

[PATCH] virtio_console: Stop doing DMA on the stack

[Andy Lutomirski]

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 
---

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, , sizeof(cpkt));
-
spin_lock(>c_ovq_lock);
-   if (virtqueue_add_outbuf(vq, sg, 1, , 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, >cpkt, sizeof(struct virtio_console_control));
+
+   if (virtqueue_add_outbuf(vq, sg, 1, >cpkt, GFP_ATOMIC) == 0) {
virtqueue_kick(vq);
while (!virtqueue_get_buf(vq, )
&& !virtqueue_is_broken(vq))
cpu_relax();
}
+
spin_unlock(>c_ovq_lock);
return 0;
 }
-- 
2.7.4

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