--- v10
+++ v7
@@ -1,222 +1,43 @@
-As well known, hvc backend can register its opertions to hvc backend.
-the operations contain put_chars(), get_chars() and so on.
+hvc framework will never pass stack memory to the put_chars() function,
+So the calling of kmemdup() is unnecessary, we can remove it.
-Some hvc backend may do dma in its operations. eg, put_chars() of
-virtio-console. But in the code of hvc framework, it may pass DMA
-incapable memory to put_chars() under a specific configuration, which
-is explained in commit c4baad5029(virtio-console: avoid DMA from stack):
-1, c[] is on stack,
- hvc_console_print():
- char c[N_OUTBUF] __ALIGNED__;
- cons_ops[index]->put_chars(vtermnos[index], c, i);
-2, ch is on stack,
- static void hvc_poll_put_char(,,char ch)
- {
- struct tty_struct *tty = driver->ttys[0];
- struct hvc_struct *hp = tty->driver_data;
- int n;
-
- do {
- n = hp->ops->put_chars(hp->vtermno, &ch, 1);
- } while (n <= 0);
- }
-
-Commit c4baad5029 is just the fix to avoid DMA from stack memory, which
-is passed to virtio-console by hvc framework in above code. But I think
-the fix is aggressive, it directly uses kmemdup() to alloc new buffer
-from kmalloc area and do memcpy no matter the memory is in kmalloc area
-or not. But most importantly, it should better be fixed in the hvc
-framework, by changing it to never pass stack memory to the put_chars()
-function in the first place. Otherwise, we still face the same issue if
-a new hvc backend using dma added in the furture.
-
-In this patch, add 'char cons_outbuf[]' as part of 'struct hvc_struct',
-so hp->cons_outbuf is no longer the stack memory, we can use it in above
-case 1. Add 'char outchar' as part of 'struct hvc_struct', we can use it
-in above case 2. We also add lock for each above buf to protect them
-separately instead of using the global lock of hvc.
-
-Introduce another array(cons_hvcs[]) for hvc pointers next to the
-cons_ops[] and vtermnos[] arrays. With the array, we can easily find
-hvc's cons_outbuf and its lock.
-
-With the patch, we can revert the fix c4baad5029.
+This revert commit c4baad5029 ("virtio-console: avoid DMA from stack")
Signed-off-by: Xianting Tian <xianting.tian@linux.alibaba.com>
-Signed-off-by: Shile Zhang <shile.zhang@linux.alibaba.com>
---
- drivers/tty/hvc/hvc_console.c | 37 +++++++++++++++++++++--------------
- drivers/tty/hvc/hvc_console.h | 24 +++++++++++++++++++++--
- 2 files changed, 44 insertions(+), 17 deletions(-)
+ drivers/char/virtio_console.c | 12 ++----------
+ 1 file changed, 2 insertions(+), 10 deletions(-)
-diff --git a/drivers/tty/hvc/hvc_console.c b/drivers/tty/hvc/hvc_console.c
-index 5bb8c4e44..4d8f112f2 100644
---- a/drivers/tty/hvc/hvc_console.c
-+++ b/drivers/tty/hvc/hvc_console.c
-@@ -41,16 +41,6 @@
- */
- #define HVC_CLOSE_WAIT (HZ/100) /* 1/10 of a second */
+diff --git a/drivers/char/virtio_console.c b/drivers/char/virtio_console.c
+index 7eaf303a7..4ed3ffb1d 100644
+--- a/drivers/char/virtio_console.c
++++ b/drivers/char/virtio_console.c
+@@ -1117,8 +1117,6 @@ static int put_chars(u32 vtermno, const char *buf, int count)
+ {
+ struct port *port;
+ struct scatterlist sg[1];
+- void *data;
+- int ret;
--/*
-- * These sizes are most efficient for vio, because they are the
-- * native transfer size. We could make them selectable in the
-- * future to better deal with backends that want other buffer sizes.
-- */
--#define N_OUTBUF 16
--#define N_INBUF 16
+ if (unlikely(early_put_chars))
+ return early_put_chars(vtermno, buf, count);
+@@ -1127,14 +1125,8 @@ static int put_chars(u32 vtermno, const char *buf, int count)
+ if (!port)
+ return -EPIPE;
+
+- data = kmemdup(buf, count, GFP_ATOMIC);
+- if (!data)
+- return -ENOMEM;
-
--#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
--
- static struct tty_driver *hvc_driver;
- static struct task_struct *hvc_task;
-
-@@ -142,6 +132,7 @@ static int hvc_flush(struct hvc_struct *hp)
- static const struct hv_ops *cons_ops[MAX_NR_HVC_CONSOLES];
- static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
- {[0 ... MAX_NR_HVC_CONSOLES - 1] = -1};
-+static struct hvc_struct *cons_hvcs[MAX_NR_HVC_CONSOLES];
+- sg_init_one(sg, data, count);
+- ret = __send_to_port(port, sg, 1, count, data, false);
+- kfree(data);
+- return ret;
++ sg_init_one(sg, buf, count);
++ return __send_to_port(port, sg, 1, count, (void *)buf, false);
+ }
/*
- * Console APIs, NOT TTY. These APIs are available immediately when
-@@ -151,9 +142,11 @@ static uint32_t vtermnos[MAX_NR_HVC_CONSOLES] =
- static void hvc_console_print(struct console *co, const char *b,
- unsigned count)
- {
-- char c[N_OUTBUF] __ALIGNED__;
-+ char *c;
- unsigned i = 0, n = 0;
- int r, donecr = 0, index = co->index;
-+ unsigned long flags;
-+ struct hvc_struct *hp;
-
- /* Console access attempt outside of acceptable console range. */
- if (index >= MAX_NR_HVC_CONSOLES)
-@@ -163,6 +156,13 @@ static void hvc_console_print(struct console *co, const char *b,
- if (vtermnos[index] == -1)
- return;
-
-+ hp = cons_hvcs[index];
-+ if (!hp)
-+ return;
-+
-+ c = hp->cons_outbuf;
-+
-+ spin_lock_irqsave(&hp->cons_outbuf_lock, flags);
- while (count > 0 || i > 0) {
- if (count > 0 && i < sizeof(c)) {
- if (b[n] == '\n' && !donecr) {
-@@ -191,6 +191,7 @@ static void hvc_console_print(struct console *co, const char *b,
- }
- }
- }
-+ spin_unlock_irqrestore(&hp->cons_outbuf_lock, flags);
- hvc_console_flush(cons_ops[index], vtermnos[index]);
- }
-
-@@ -878,9 +879,13 @@ static void hvc_poll_put_char(struct tty_driver *driver, int line, char ch)
- struct tty_struct *tty = driver->ttys[0];
- struct hvc_struct *hp = tty->driver_data;
- int n;
-+ unsigned long flags;
-
- do {
-- n = hp->ops->put_chars(hp->vtermno, &ch, 1);
-+ spin_lock_irqsave(&hp->outchar_lock, flags);
-+ hp->outchar = ch;
-+ n = hp->ops->put_chars(hp->vtermno, hp->outchar, 1);
-+ spin_unlock_irqrestore(&hp->outchar_lock, flags);
- } while (n <= 0);
- }
- #endif
-@@ -922,8 +927,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
- return ERR_PTR(err);
- }
-
-- hp = kzalloc(ALIGN(sizeof(*hp), sizeof(long)) + outbuf_size,
-- GFP_KERNEL);
-+ hp = kzalloc(struct_size(hp, outbuf, outbuf_size), GFP_KERNEL);
- if (!hp)
- return ERR_PTR(-ENOMEM);
-
-@@ -931,13 +935,14 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
- hp->data = data;
- hp->ops = ops;
- hp->outbuf_size = outbuf_size;
-- hp->outbuf = &((char *)hp)[ALIGN(sizeof(*hp), sizeof(long))];
-
- tty_port_init(&hp->port);
- hp->port.ops = &hvc_port_ops;
-
- INIT_WORK(&hp->tty_resize, hvc_set_winsz);
- spin_lock_init(&hp->lock);
-+ spin_lock_init(&hp->outchar_lock);
-+ spin_lock_init(&hp->cons_outbuf_lock);
- mutex_lock(&hvc_structs_mutex);
-
- /*
-@@ -964,6 +969,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
- if (i < MAX_NR_HVC_CONSOLES) {
- cons_ops[i] = ops;
- vtermnos[i] = vtermno;
-+ cons_hvcs[i] = hp;
- }
-
- list_add_tail(&(hp->next), &hvc_structs);
-@@ -988,6 +994,7 @@ int hvc_remove(struct hvc_struct *hp)
- if (hp->index < MAX_NR_HVC_CONSOLES) {
- vtermnos[hp->index] = -1;
- cons_ops[hp->index] = NULL;
-+ cons_hvcs[hp->index] = NULL;
- }
-
- /* Don't whack hp->irq because tty_hangup() will need to free the irq. */
-diff --git a/drivers/tty/hvc/hvc_console.h b/drivers/tty/hvc/hvc_console.h
-index 18d005814..98f0ced83 100644
---- a/drivers/tty/hvc/hvc_console.h
-+++ b/drivers/tty/hvc/hvc_console.h
-@@ -32,13 +32,21 @@
- */
- #define HVC_ALLOC_TTY_ADAPTERS 8
-
-+/*
-+ * These sizes are most efficient for vio, because they are the
-+ * native transfer size. We could make them selectable in the
-+ * future to better deal with backends that want other buffer sizes.
-+ */
-+#define N_OUTBUF 16
-+#define N_INBUF 16
-+
-+#define __ALIGNED__ __attribute__((__aligned__(sizeof(long))))
-+
- struct hvc_struct {
- struct tty_port port;
- spinlock_t lock;
- int index;
- int do_wakeup;
-- char *outbuf;
-- int outbuf_size;
- int n_outbuf;
- uint32_t vtermno;
- const struct hv_ops *ops;
-@@ -48,6 +56,18 @@ struct hvc_struct {
- struct work_struct tty_resize;
- struct list_head next;
- unsigned long flags;
-+
-+ /* the buf is used in hvc console api for putting chars */
-+ char cons_outbuf[N_OUTBUF] __ALIGNED__;
-+ spinlock_t cons_outbuf_lock;
-+
-+ /* the buf is for putting single char to tty */
-+ char outchar;
-+ spinlock_t outchar_lock;
-+
-+ /* the buf is for putting chars to tty */
-+ int outbuf_size;
-+ char outbuf[0] __ALIGNED__;
- };
-
- /* implemented by a low level driver */
--
2.17.1