hvc and powerpc opal console latency reduction

PATCH Applied Nicholas Piggin 4 participants 31 messages Apr 30, 2018 – Aug 21, 2018

Subsystems (3)

PowerPC TTY layer and serial drivers Hypervisor virtual console driver

lore

Share & tools

lore

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #0 lore raw

cover hvc and powerpc opal console latency reduction

I'm seeing scattered reports of hard lockups triggering in the OPAL console code. I haven't got a full latency trace -- they are difficult to reproduce and sometimes just show up in dmesg of a bug report when the system is having other issues. But it does seem like there are some improvements that can be made to the hvc console and powerpc opal driver.

I'm not sure how this series should be merged yet, but we can cross that if/when it comes. Possibly all can go via the tty tree with acks from powerpc maintainers.

Comments appreciated.

Thanks,
Nick

--
Nicholas Piggin (15):
powerpc/powernv: opal_put_chars partial write fix powerpc/powernv: Fix OPAL console driver OPAL_BUSY loops powerpc/powernv: opal-kmsg standardise OPAL_BUSY handling powerpc/powernv: opal-kmsg use flush fallback from console code powerpc/powernv: Implement and use opal_flush_console powerpc/powernv: Remove OPALv1 support from opal console driver powerpc/powernv: move opal console flushing to udbg powerpc/powernv: implement opal_put_chars_atomic
tty: hvc: remove unexplained "just in case" spin delay
tty: hvc: use mutex instead of spinlock for hvc_structs lock
tty: hvc: hvc_poll break hv read loop
tty: hvc: hvc_poll may sleep
tty: hvc: hvc_write may sleep
tty: hvc: introduce the hv_ops.flush operation for hvc drivers powerpc/powernv: provide a console flush operation for opal hvc driver

 arch/powerpc/include/asm/opal.h            |   3 +
 arch/powerpc/platforms/powernv/opal-kmsg.c |  30 +---
 arch/powerpc/platforms/powernv/opal.c      | 176 +++++++++++++------
 drivers/tty/hvc/hvc_console.c              | 194 +++++++++++++--------
 drivers/tty/hvc/hvc_console.h              |   1 +
 drivers/tty/hvc/hvc_opal.c                 |  33 ++--
 6 files changed, 276 insertions(+), 161 deletions(-)

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #1 lore raw

PATCH 1/15 powerpc/powernv: opal_put_chars partial write fix

The intention here is to consume and discard the remaining buffer upon error. This works if there has not been a previous partial write. If there has been, then total_len is no longer total number of bytes to copy. total_len is always "bytes left to copy", so it should be added to written bytes.

This code may not be exercised any more if partial writes will not be hit, but this is a small bugfix before a larger change.

Reviewed-by Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/platforms/powernv/opal.c +1 -1

index 48fbb41af5d1..e695b836fd49 100644
@@ -388,7 +388,7 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 		/* Closed or other error drop */
 		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
 		    rc != OPAL_BUSY_EVENT) {
-			written = total_len;
+			written += total_len;
 			break;
 		}
 		if (rc == OPAL_SUCCESS) {

Signature · 1 line

2.17.0

Michael Ellerman

2018-07-24 13:59 UTC #2 lore raw

Re: [01/15] powerpc/powernv: opal_put_chars partial write fix

Quoted 11 lines · Nicholas Piggin

The intention here is to consume and discard the remaining buffer upon error. This works if there has not been a previous partial write. If there has been, then total_len is no longer total number of bytes to copy. total_len is always "bytes left to copy", so it should be added to written bytes.
This code may not be exercised any more if partial writes will not be hit, but this is a small bugfix before a larger change.
Reviewed-by Benjamin Herrenschmidt <[email protected]>
Signed-off-by Nicholas Piggin <[email protected]>

Patches 1-9 applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/bd90284cc6c1c9e8e48c8eadd0c795

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #3 lore raw

PATCH 2/15 powerpc/powernv: Fix OPAL console driver OPAL_BUSY loops

In reply to Nicholas Piggin

The OPAL console driver does not delay in case it gets OPAL_BUSY or OPAL_BUSY_EVENT from firmware.

It can't yet be made to sleep because it is called under spinlock, but it can be changed to the standard OPAL_BUSY loop form, and a delay added to keep it from hitting the firmware too frequently.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 38 ++++++++++++++++-----------
 1 file changed, 23 insertions(+), 15 deletions(-)

arch/powerpc/platforms/powernv/opal.c +23 ~~-15~~

index e695b836fd49..6b621d47ac29 100644
@@ -378,33 +378,41 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	/* We still try to handle partial completions, though they
 	 * should no longer happen.
 	 */
-	rc = OPAL_BUSY;
-	while(total_len > 0 && (rc == OPAL_BUSY ||
-				rc == OPAL_BUSY_EVENT || rc == OPAL_SUCCESS)) {
+
+	while (total_len > 0) {
 		olen = cpu_to_be64(total_len);
-		rc = opal_console_write(vtermno, &olen, data);
+
+		rc = OPAL_BUSY;
+		while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+			rc = opal_console_write(vtermno, &olen, data);
+			if (rc == OPAL_BUSY_EVENT) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+				opal_poll_events(NULL);
+			} else if (rc == OPAL_BUSY) {
+				mdelay(OPAL_BUSY_DELAY_MS);
+			}
+		}
+
 		len = be64_to_cpu(olen);
 
 		/* Closed or other error drop */
-		if (rc != OPAL_SUCCESS && rc != OPAL_BUSY &&
-		    rc != OPAL_BUSY_EVENT) {
-			written += total_len;
+		if (rc != OPAL_SUCCESS) {
+			written += total_len; /* drop remaining chars */
 			break;
 		}
-		if (rc == OPAL_SUCCESS) {
-			total_len -= len;
-			data += len;
-			written += len;
-		}
+
+		total_len -= len;
+		data += len;
+		written += len;
+
 		/* This is a bit nasty but we need that for the console to
 		 * flush when there aren't any interrupts. We will clean
 		 * things a bit later to limit that to synchronous path
 		 * such as the kernel console and xmon/udbg
 		 */
-		do
+		do {
 			opal_poll_events(&evt);
-		while(rc == OPAL_SUCCESS &&
-			(be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT));
+		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
 	}
 	spin_unlock_irqrestore(&opal_write_lock, flags);
 	return written;

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #4 lore raw

PATCH 3/15 powerpc/powernv: opal-kmsg standardise OPAL_BUSY handling

In reply to Nicholas Piggin

OPAL_CONSOLE_FLUSH is documented as being able to return OPAL_BUSY, so implement the standard OPAL_BUSY handling for it.

Reviewed-by Russell Currey <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-kmsg.c | 24 ++++++++++++++--------
 1 file changed, 16 insertions(+), 8 deletions(-)

arch/powerpc/platforms/powernv/opal-kmsg.c +16 -8

index 6f1214d4de92..f8f41ccce75f 100644
@@ -12,6 +12,7 @@
  */
 
 #include <linux/kmsg_dump.h>
+#include <linux/delay.h>
 
 #include <asm/opal.h>
 #include <asm/opal-api.h>

@@ -26,8 +27,7 @@
 static void force_opal_console_flush(struct kmsg_dumper *dumper,
 				     enum kmsg_dump_reason reason)
 {
-	int i;
-	int64_t ret;
+	s64 rc;
 
 	/*
 	 * Outside of a panic context the pollers will continue to run,

@@ -37,14 +37,22 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper,
 		return;
 
 	if (opal_check_token(OPAL_CONSOLE_FLUSH)) {
-		ret = opal_console_flush(0);
+		do  {
+			rc = OPAL_BUSY;
+			while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+				rc = opal_console_flush(0);
+				if (rc == OPAL_BUSY_EVENT) {
+					mdelay(OPAL_BUSY_DELAY_MS);
+					opal_poll_events(NULL);
+				} else if (rc == OPAL_BUSY) {
+					mdelay(OPAL_BUSY_DELAY_MS);
+				}
+			}
+		} while (rc == OPAL_PARTIAL); /* More to flush */
 
-		if (ret == OPAL_UNSUPPORTED || ret == OPAL_PARAMETER)
-			return;
-
-		/* Incrementally flush until there's nothing left */
-		while (opal_console_flush(0) != OPAL_SUCCESS);
 	} else {
+		int i;
+
 		/*
 		 * If OPAL_CONSOLE_FLUSH is not implemented in the firmware,
 		 * the console can still be flushed by calling the polling

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #5 lore raw

PATCH 4/15 powerpc/powernv: opal-kmsg use flush fallback from console code

In reply to Nicholas Piggin

Use the more refined and tested event polling loop from opal_put_chars as the fallback console flush in the opal-kmsg path. This loop is used by the console driver today, whereas the opal-kmsg fallback is not likely to have been used for years.

Use WARN_ONCE rather than a printk when the fallback is invoked to prepare for moving the console flush into a common function.

Reviewed-by Russell Currey <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal-kmsg.c | 15 ++++++---------
 1 file changed, 6 insertions(+), 9 deletions(-)

arch/powerpc/platforms/powernv/opal-kmsg.c +6 -9

index f8f41ccce75f..fd2bbf4fd6dc 100644
@@ -51,20 +51,17 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper,
 		} while (rc == OPAL_PARTIAL); /* More to flush */
 
 	} else {
-		int i;
+		__be64 evt;
 
+		WARN_ONCE(1, "opal: OPAL_CONSOLE_FLUSH missing.\n");
 		/*
 		 * If OPAL_CONSOLE_FLUSH is not implemented in the firmware,
 		 * the console can still be flushed by calling the polling
-		 * function enough times to flush the buffer.  We don't know
-		 * how much output still needs to be flushed, but we can be
-		 * generous since the kernel is in panic and doesn't need
-		 * to do much else.
+		 * function while it has OPAL_EVENT_CONSOLE_OUTPUT events.
 		 */
-		printk(KERN_NOTICE "opal: OPAL_CONSOLE_FLUSH missing.\n");
-		for (i = 0; i < 1024; i++) {
-			opal_poll_events(NULL);
-		}
+		do {
+			opal_poll_events(&evt);
+		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
 	}
 }

Signature · 1 line

2.17.0

Michael Ellerman

reviewer

2018-05-04 05:16 UTC #6 lore raw

Re: [PATCH 04/15] powerpc/powernv: opal-kmsg use flush fallback from console code

Quoted 7 lines · Nicholas Piggin

Use the more refined and tested event polling loop from opal_put_chars as the fallback console flush in the opal-kmsg path. This loop is used by the console driver today, whereas the opal-kmsg fallback is not likely to have been used for years.
Use WARN_ONCE rather than a printk when the fallback is invoked to prepare for moving the console flush into a common function.

Do we want to add a WARN in that path? If we're panicking things might get worse if we WARN (which takes a trap).

cheers

Nicholas Piggin

author reviewer

2018-05-04 05:37 UTC #7 lore raw

Re: [PATCH 04/15] powerpc/powernv: opal-kmsg use flush fallback from console code

Quoted 7 lines · On Fri, 04 May 2018 15:16:37 +1000 Michael Ellerman <[email protected]> wrote:

Quoted 5 lines · Nicholas Piggin
Use the more refined and tested event polling loop from opal_put_chars as the fallback console flush in the opal-kmsg path. This loop is used by the console driver today, whereas the opal-kmsg fallback is not likely to have been used for years.

Quoted 5 lines

Quoted 2 lines
Use WARN_ONCE rather than a printk when the fallback is invoked to prepare for moving the console flush into a common function.
Do we want to add a WARN in that path? If we're panicking things might get worse if we WARN (which takes a trap).

True, probably a good idea not to... oh there's a printk_once so that'll work nicely.

Thanks,
Nick

Michael Ellerman

reviewer

2018-05-07 10:36 UTC #8 lore raw

Re: [PATCH 04/15] powerpc/powernv: opal-kmsg use flush fallback from console code

Quoted 8 lines · Nicholas Piggin

Quoted 6 lines · On Fri, 04 May 2018 15:16:37 +1000 Michael Ellerman <[email protected]> wrote:
Quoted 5 lines · Nicholas Piggin
Use the more refined and tested event polling loop from opal_put_chars as the fallback console flush in the opal-kmsg path. This loop is used by the console driver today, whereas the opal-kmsg fallback is not likely to have been used for years.

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
Use WARN_ONCE rather than a printk when the fallback is invoked to prepare for moving the console flush into a common function.
Do we want to add a WARN in that path? If we're panicking things might get worse if we WARN (which takes a trap).
True, probably a good idea not to... oh there's a printk_once so that'll work nicely.

Cool.

I have this series in a tree so you can send me an incremental diff if it's reasonably small.

cheers

Nicholas Piggin

author reviewer

2018-05-08 03:40 UTC #9 lore raw

Re: [PATCH 04/15] powerpc/powernv: opal-kmsg use flush fallback from console code

Quoted 15 lines · On Mon, 07 May 2018 20:36:39 +1000 Michael Ellerman <[email protected]> wrote:

Quoted 14 lines · Nicholas Piggin
Quoted 11 lines · On Fri, 04 May 2018 15:16:37 +1000 Michael Ellerman <[email protected]> wrote:
Quoted 7 lines · Nicholas Piggin
Use the more refined and tested event polling loop from opal_put_chars as the fallback console flush in the opal-kmsg path. This loop is used by the console driver today, whereas the opal-kmsg fallback is not likely to have been used for years.
Use WARN_ONCE rather than a printk when the fallback is invoked to prepare for moving the console flush into a common function.
Do we want to add a WARN in that path? If we're panicking things might get worse if we WARN (which takes a trap).

Quoted 7 lines

Quoted 2 lines
True, probably a good idea not to... oh there's a printk_once so that'll work nicely.
Cool.
I have this series in a tree so you can send me an incremental diff if it's reasonably small.

It's a one liner (also moved location of message back to where it was originally).

The next patch will clash because it moves this over into opal.c, so you'd have to fix that by hand.

Thanks,
Nick

---
 arch/powerpc/platforms/powernv/opal-kmsg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

arch/powerpc/platforms/powernv/opal-kmsg.c +1 -1

index fd2bbf4fd6dc..c610ef3541aa 100644
@@ -53,12 +53,12 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper,
 	} else {
 		__be64 evt;
 
-		WARN_ONCE(1, "opal: OPAL_CONSOLE_FLUSH missing.\n");
 		/*
 		 * If OPAL_CONSOLE_FLUSH is not implemented in the firmware,
 		 * the console can still be flushed by calling the polling
 		 * function while it has OPAL_EVENT_CONSOLE_OUTPUT events.
 		 */
+		printk_once(KERN_NOTICE "opal: OPAL_CONSOLE_FLUSH missing.\n");
 		do {
 			opal_poll_events(&evt);
 		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #10 lore raw

PATCH 5/15 powerpc/powernv: Implement and use opal_flush_console

In reply to Nicholas Piggin

A new console flushing firmware API was introduced to replace event polling loops, and implemented in opal-kmsg with affddff69c55e ("powerpc/powernv: Add a kmsg_dumper that flushes console output on panic"), to flush the console in the panic path.

The OPAL console driver has other situations where interrupts are off and it needs to flush the console synchronously. These still use a polling loop.

So move the opal-kmsg flush code to opal_flush_console, and use the new function in opal-kmsg and opal_put_chars.

Cc Benjamin Herrenschmidt <[email protected]>

Reviewed-by Russell Currey <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h            |  1 +
 arch/powerpc/platforms/powernv/opal-kmsg.c | 35 ++----------------
 arch/powerpc/platforms/powernv/opal.c      | 42 +++++++++++++++++++---
 3 files changed, 42 insertions(+), 36 deletions(-)

arch/powerpc/include/asm/opal.h +1 -0

index 03e1a920491e..bbff49fab0e5 100644
@@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
 
 extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
 extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
+extern int opal_flush_console(uint32_t vtermno);
 
 extern void hvc_opal_init_early(void);

arch/powerpc/platforms/powernv/opal-kmsg.c +3 ~~-32~~

index fd2bbf4fd6dc..55691950d981 100644
@@ -12,7 +12,6 @@
  */
 
 #include <linux/kmsg_dump.h>
-#include <linux/delay.h>
 
 #include <asm/opal.h>
 #include <asm/opal-api.h>

@@ -24,11 +23,9 @@
  * may not be completely printed.  This function does not actually dump the
  * message, it just ensures that OPAL completely flushes the console buffer.
  */
-static void force_opal_console_flush(struct kmsg_dumper *dumper,
+static void kmsg_dump_opal_console_flush(struct kmsg_dumper *dumper,
 				     enum kmsg_dump_reason reason)
 {
-	s64 rc;
-
 	/*
 	 * Outside of a panic context the pollers will continue to run,
 	 * so we don't need to do any special flushing.

@@ -36,37 +33,11 @@ static void force_opal_console_flush(struct kmsg_dumper *dumper,
 	if (reason != KMSG_DUMP_PANIC)
 		return;
 
-	if (opal_check_token(OPAL_CONSOLE_FLUSH)) {
-		do  {
-			rc = OPAL_BUSY;
-			while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
-				rc = opal_console_flush(0);
-				if (rc == OPAL_BUSY_EVENT) {
-					mdelay(OPAL_BUSY_DELAY_MS);
-					opal_poll_events(NULL);
-				} else if (rc == OPAL_BUSY) {
-					mdelay(OPAL_BUSY_DELAY_MS);
-				}
-			}
-		} while (rc == OPAL_PARTIAL); /* More to flush */
-
-	} else {
-		__be64 evt;
-
-		WARN_ONCE(1, "opal: OPAL_CONSOLE_FLUSH missing.\n");
-		/*
-		 * If OPAL_CONSOLE_FLUSH is not implemented in the firmware,
-		 * the console can still be flushed by calling the polling
-		 * function while it has OPAL_EVENT_CONSOLE_OUTPUT events.
-		 */
-		do {
-			opal_poll_events(&evt);
-		} while (be64_to_cpu(evt) & OPAL_EVENT_CONSOLE_OUTPUT);
-	}
+	opal_flush_console(0);
 }
 
 static struct kmsg_dumper opal_kmsg_dumper = {
-	.dump = force_opal_console_flush
+	.dump = kmsg_dump_opal_console_flush
 };
 
 void __init opal_kmsg_init(void)

arch/powerpc/platforms/powernv/opal.c +38 -4

64 lines not on this page · raw message

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #11 lore raw

PATCH 6/15 powerpc/powernv: Remove OPALv1 support from opal console driver

In reply to Nicholas Piggin

opal_put_chars deals with partial writes because in OPALv1, opal_console_write_buffer_space did not work correctly. That firmware is not supported.

This reworks the opal_put_chars code to no longer deal with partial writes by turning them into full writes. Partial write handling is still supported in terms of what gets returned to the caller, but it may not go to the console atomically. A warning message is printed in this case.

This allows console flushing to be moved out of the opal_write_lock spinlock. That could cause the lock to be held for long periods if the console is busy (especially if it was being spammed by firmware), which is dangerous because the lock is taken by xmon to debug the system. Flushing outside the lock improves the situation a bit.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 86 +++++++++++++--------------
 1 file changed, 40 insertions(+), 46 deletions(-)

arch/powerpc/platforms/powernv/opal.c +40 ~~-46~~

index 6640fccbf30c..0ddb63226695 100644
@@ -346,10 +346,10 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
 
 int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 {
-	int written = 0;
-	__be64 olen;
-	s64 len, rc;
 	unsigned long flags;
+	int written;
+	__be64 olen;
+	s64 rc;
 
 	if (!opal.entry)
 		return -ENODEV;

@@ -357,62 +357,56 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	/* We want put_chars to be atomic to avoid mangling of hvsi
 	 * packets. To do that, we first test for room and return
 	 * -EAGAIN if there isn't enough.
-	 *
-	 * Unfortunately, opal_console_write_buffer_space() doesn't
-	 * appear to work on opal v1, so we just assume there is
-	 * enough room and be done with it
 	 */
 	spin_lock_irqsave(&opal_write_lock, flags);
 	rc = opal_console_write_buffer_space(vtermno, &olen);
-	len = be64_to_cpu(olen);
-	if (rc || len < total_len) {
-		spin_unlock_irqrestore(&opal_write_lock, flags);
+	if (rc || be64_to_cpu(olen) < total_len) {
 		/* Closed -> drop characters */
 		if (rc)
-			return total_len;
-		opal_flush_console(vtermno);
-		return -EAGAIN;
+			written = total_len;
+		else
+			written = -EAGAIN;
+		goto out;
 	}
 
-	/* We still try to handle partial completions, though they
-	 * should no longer happen.
-	 */
-
-	while (total_len > 0) {
-		olen = cpu_to_be64(total_len);
-
-		rc = OPAL_BUSY;
-		while (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
-			rc = opal_console_write(vtermno, &olen, data);
-			if (rc == OPAL_BUSY_EVENT) {
-				mdelay(OPAL_BUSY_DELAY_MS);
-				opal_poll_events(NULL);
-			} else if (rc == OPAL_BUSY) {
-				mdelay(OPAL_BUSY_DELAY_MS);
-			}
-		}
-
-		len = be64_to_cpu(olen);
-
-		/* Closed or other error drop */
-		if (rc != OPAL_SUCCESS) {
-			written += total_len; /* drop remaining chars */
-			break;
+	/* Should not get a partial write here because space is available. */
+	olen = cpu_to_be64(total_len);
+	rc = opal_console_write(vtermno, &olen, data);
+	if (rc == OPAL_BUSY || rc == OPAL_BUSY_EVENT) {
+		if (rc == OPAL_BUSY_EVENT) {
+			mdelay(OPAL_BUSY_DELAY_MS);
+			opal_poll_events(NULL);
+		} else if (rc == OPAL_BUSY_EVENT) {
+			mdelay(OPAL_BUSY_DELAY_MS);
 		}
+		written = -EAGAIN;
+		goto out;
+	}
 
-		total_len -= len;
-		data += len;
-		written += len;
+	/* Closed or other error drop */
+	if (rc != OPAL_SUCCESS) {
+		written = opal_error_code(rc);
+		goto out;
+	}
 
-		/* This is a bit nasty but we need that for the console to
-		 * flush when there aren't any interrupts. We will clean
-		 * things a bit later to limit that to synchronous path
-		 * such as the kernel console and xmon/udbg
-		 */
-		opal_flush_console(vtermno);
+	written = be64_to_cpu(olen);
+	if (written < total_len) {
+		/* Should not happen */
+		pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
+		if (!written)
+			written = -EAGAIN;
 	}
+
+out:
 	spin_unlock_irqrestore(&opal_write_lock, flags);
 
+	/* This is a bit nasty but we need that for the console to
+	 * flush when there aren't any interrupts. We will clean
+	 * things a bit later to limit that to synchronous path
+	 * such as the kernel console and xmon/udbg
+	 */
+	opal_flush_console(vtermno);
+
 	return written;
 }

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #12 lore raw

PATCH 7/15 powerpc/powernv: move opal console flushing to udbg

In reply to Nicholas Piggin

OPAL console writes do not have to synchronously flush firmware / hardware buffers unless they are going through the udbg path.

Remove the unconditional flushing from opal_put_chars. Flush if there was no space in the buffer as an optimisation (callers loop waiting for success in that case). udbg flushing is moved to udbg_opal_putc.

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/platforms/powernv/opal.c | 12 +++++++-----
 drivers/tty/hvc/hvc_opal.c            |  5 +++++
 2 files changed, 12 insertions(+), 5 deletions(-)

arch/powerpc/platforms/powernv/opal.c +7 -5

index 0ddb63226695..55d4b1983110 100644
@@ -400,12 +400,14 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 out:
 	spin_unlock_irqrestore(&opal_write_lock, flags);
 
-	/* This is a bit nasty but we need that for the console to
-	 * flush when there aren't any interrupts. We will clean
-	 * things a bit later to limit that to synchronous path
-	 * such as the kernel console and xmon/udbg
+	/* In the -EAGAIN case, callers loop, so we have to flush the console
+	 * here in case they have interrupts off (and we don't want to wait
+	 * for async flushing if we can make immediate progress here). If
+	 * necessary the API could be made entirely non-flushing if the
+	 * callers had a ->flush API to use.
 	 */
-	opal_flush_console(vtermno);
+	if (written == -EAGAIN)
+		opal_flush_console(vtermno);
 
 	return written;
 }

drivers/tty/hvc/hvc_opal.c +5 -0

index 2ed07ca6389e..af122ad7f06d 100644
@@ -275,6 +275,11 @@ static void udbg_opal_putc(char c)
 			count = hvc_opal_hvsi_put_chars(termno, &c, 1);
 			break;
 		}
+
+		/* This is needed for the cosole to flush
+		 * when there aren't any interrupts.
+		 */
+		opal_flush_console(termno);
 	} while(count == 0 || count == -EAGAIN);
 }

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #13 lore raw

PATCH 8/15 powerpc/powernv: implement opal_put_chars_atomic

In reply to Nicholas Piggin

The RAW console does not need writes to be atomic, so relax opal_put_chars to be able to do partial writes, and implement an _atomic variant which does not take a spinlock. This API is used in xmon, so the less locking that is used, the better chance there is that a crash can be debugged.

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h       |  1 +
 arch/powerpc/platforms/powernv/opal.c | 37 +++++++++++++++++++--------
 drivers/tty/hvc/hvc_opal.c            | 18 +++++++++----
 3 files changed, 41 insertions(+), 15 deletions(-)

arch/powerpc/include/asm/opal.h +1 -0

index bbff49fab0e5..5d7072411561 100644
@@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
 
 extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
 extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
+extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
 extern int opal_flush_console(uint32_t vtermno);
 
 extern void hvc_opal_init_early(void);

arch/powerpc/platforms/powernv/opal.c +27 ~~-10~~

index 55d4b1983110..bcdb90ada938 100644
@@ -344,9 +344,9 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
 	return 0;
 }
 
-int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+static int __opal_put_chars(uint32_t vtermno, const char *data, int total_len, bool atomic)
 {
-	unsigned long flags;
+	unsigned long flags = 0 /* shut up gcc */;
 	int written;
 	__be64 olen;
 	s64 rc;

@@ -354,11 +354,8 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	if (!opal.entry)
 		return -ENODEV;
 
-	/* We want put_chars to be atomic to avoid mangling of hvsi
-	 * packets. To do that, we first test for room and return
-	 * -EAGAIN if there isn't enough.
-	 */
-	spin_lock_irqsave(&opal_write_lock, flags);
+	if (atomic)
+		spin_lock_irqsave(&opal_write_lock, flags);
 	rc = opal_console_write_buffer_space(vtermno, &olen);
 	if (rc || be64_to_cpu(olen) < total_len) {
 		/* Closed -> drop characters */

@@ -391,14 +388,18 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 
 	written = be64_to_cpu(olen);
 	if (written < total_len) {
-		/* Should not happen */
-		pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
+		if (atomic) {
+			/* Should not happen */
+			pr_warn("atomic console write returned partial "
+				"len=%d written=%d\n", total_len, written);
+		}
 		if (!written)
 			written = -EAGAIN;
 	}
 
 out:
-	spin_unlock_irqrestore(&opal_write_lock, flags);
+	if (atomic)
+		spin_unlock_irqrestore(&opal_write_lock, flags);
 
 	/* In the -EAGAIN case, callers loop, so we have to flush the console
 	 * here in case they have interrupts off (and we don't want to wait

@@ -412,6 +413,22 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	return written;
 }
 
+int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, false);
+}
+
+/*
+ * opal_put_chars_atomic will not perform partial-writes. Data will be
+ * atomically written to the terminal or not at all. This is not strictly
+ * true at the moment because console space can race with OPAL's console
+ * writes.
+ */
+int opal_put_chars_atomic(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, true);
+}
+
 int opal_flush_console(uint32_t vtermno)
 {
 	s64 rc;

drivers/tty/hvc/hvc_opal.c +13 -5

index af122ad7f06d..0a72f98ee082 100644
@@ -183,9 +183,15 @@ static int hvc_opal_probe(struct platform_device *dev)
 			return -ENOMEM;
 		pv->proto = proto;
 		hvc_opal_privs[termno] = pv;
-		if (proto == HV_PROTOCOL_HVSI)
-			hvsilib_init(&pv->hvsi, opal_get_chars, opal_put_chars,
+		if (proto == HV_PROTOCOL_HVSI) {
+			/*
+			 * We want put_chars to be atomic to avoid mangling of
+			 * hvsi packets.
+			 */
+			hvsilib_init(&pv->hvsi,
+				     opal_get_chars, opal_put_chars_atomic,
 				     termno, 0);
+		}
 
 		/* Instanciate now to establish a mapping index==vtermno */
 		hvc_instantiate(termno, termno, ops);

@@ -376,8 +382,9 @@ void __init hvc_opal_init_early(void)
 	else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
 		hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
 		ops = &hvc_opal_hvsi_ops;
-		hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars,
-			     opal_put_chars, index, 1);
+		hvsilib_init(&hvc_opal_boot_priv.hvsi,
+			     opal_get_chars, opal_put_chars_atomic,
+			     index, 1);
 		/* HVSI, perform the handshake now */
 		hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 		pr_devel("hvc_opal: Found HVSI console\n");

@@ -409,7 +416,8 @@ void __init udbg_init_debug_opal_hvsi(void)
 	hvc_opal_privs[index] = &hvc_opal_boot_priv;
 	hvc_opal_boot_termno = index;
 	udbg_init_opal_common();
-	hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars, opal_put_chars,
+	hvsilib_init(&hvc_opal_boot_priv.hvsi,
+		     opal_get_chars, opal_put_chars_atomic,
 		     index, 1);
 	hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 }

Signature · 1 line

2.17.0

Benjamin Herrenschmidt

2018-05-01 09:48 UTC #14 lore raw

Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic

Quoted 5 lines · Nicholas Piggin

The RAW console does not need writes to be atomic, so relax opal_put_chars to be able to do partial writes, and implement an _atomic variant which does not take a spinlock. This API is used in xmon, so the less locking that is used, the better chance there is that a crash can be debugged.

Same comment I already had :-) "atomic" in Linux tends to mean something else (ie, atomic context), so I'd rather have something like opal_put_chars_sync() or such...

Quoted 139 lines

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h       |  1 +
 arch/powerpc/platforms/powernv/opal.c | 37 +++++++++++++++++++--------
 drivers/tty/hvc/hvc_opal.c            | 18 +++++++++----
 3 files changed, 41 insertions(+), 15 deletions(-)

arch/powerpc/include/asm/opal.h +1 -0

index bbff49fab0e5..5d7072411561 100644
@@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
 
 extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
 extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
+extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
 extern int opal_flush_console(uint32_t vtermno);
 
 extern void hvc_opal_init_early(void);

arch/powerpc/platforms/powernv/opal.c +27 ~~-10~~

index 55d4b1983110..bcdb90ada938 100644
@@ -344,9 +344,9 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
 	return 0;
 }
 
-int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+static int __opal_put_chars(uint32_t vtermno, const char *data, int total_len, bool atomic)
 {
-	unsigned long flags;
+	unsigned long flags = 0 /* shut up gcc */;
 	int written;
 	__be64 olen;
 	s64 rc;

@@ -354,11 +354,8 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	if (!opal.entry)
 		return -ENODEV;
 
-	/* We want put_chars to be atomic to avoid mangling of hvsi
-	 * packets. To do that, we first test for room and return
-	 * -EAGAIN if there isn't enough.
-	 */
-	spin_lock_irqsave(&opal_write_lock, flags);
+	if (atomic)
+		spin_lock_irqsave(&opal_write_lock, flags);
 	rc = opal_console_write_buffer_space(vtermno, &olen);
 	if (rc || be64_to_cpu(olen) < total_len) {
 		/* Closed -> drop characters */

@@ -391,14 +388,18 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 
 	written = be64_to_cpu(olen);
 	if (written < total_len) {
-		/* Should not happen */
-		pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
+		if (atomic) {
+			/* Should not happen */
+			pr_warn("atomic console write returned partial "
+				"len=%d written=%d\n", total_len, written);
+		}
 		if (!written)
 			written = -EAGAIN;
 	}
 
 out:
-	spin_unlock_irqrestore(&opal_write_lock, flags);
+	if (atomic)
+		spin_unlock_irqrestore(&opal_write_lock, flags);
 
 	/* In the -EAGAIN case, callers loop, so we have to flush the console
 	 * here in case they have interrupts off (and we don't want to wait

@@ -412,6 +413,22 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	return written;
 }
 
+int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, false);
+}
+
+/*
+ * opal_put_chars_atomic will not perform partial-writes. Data will be
+ * atomically written to the terminal or not at all. This is not strictly
+ * true at the moment because console space can race with OPAL's console
+ * writes.
+ */
+int opal_put_chars_atomic(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, true);
+}
+
 int opal_flush_console(uint32_t vtermno)
 {
 	s64 rc;

drivers/tty/hvc/hvc_opal.c +13 -5

index af122ad7f06d..0a72f98ee082 100644
@@ -183,9 +183,15 @@ static int hvc_opal_probe(struct platform_device *dev)
 			return -ENOMEM;
 		pv->proto = proto;
 		hvc_opal_privs[termno] = pv;
-		if (proto == HV_PROTOCOL_HVSI)
-			hvsilib_init(&pv->hvsi, opal_get_chars, opal_put_chars,
+		if (proto == HV_PROTOCOL_HVSI) {
+			/*
+			 * We want put_chars to be atomic to avoid mangling of
+			 * hvsi packets.
+			 */
+			hvsilib_init(&pv->hvsi,
+				     opal_get_chars, opal_put_chars_atomic,
 				     termno, 0);
+		}
 
 		/* Instanciate now to establish a mapping index==vtermno */
 		hvc_instantiate(termno, termno, ops);

@@ -376,8 +382,9 @@ void __init hvc_opal_init_early(void)
 	else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
 		hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
 		ops = &hvc_opal_hvsi_ops;
-		hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars,
-			     opal_put_chars, index, 1);
+		hvsilib_init(&hvc_opal_boot_priv.hvsi,
+			     opal_get_chars, opal_put_chars_atomic,
+			     index, 1);
 		/* HVSI, perform the handshake now */
 		hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 		pr_devel("hvc_opal: Found HVSI console\n");

@@ -409,7 +416,8 @@ void __init udbg_init_debug_opal_hvsi(void)
 	hvc_opal_privs[index] = &hvc_opal_boot_priv;
 	hvc_opal_boot_termno = index;
 	udbg_init_opal_common();
-	hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars, opal_put_chars,
+	hvsilib_init(&hvc_opal_boot_priv.hvsi,
+		     opal_get_chars, opal_put_chars_atomic,
 		     index, 1);
 	hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 }

Nicholas Piggin

author reviewer

2018-05-01 10:37 UTC #15 lore raw

Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic

Quoted 10 lines · On Tue, 01 May 2018 19:48:58 +1000 Benjamin Herrenschmidt…

Quoted 5 lines · Nicholas Piggin
The RAW console does not need writes to be atomic, so relax opal_put_chars to be able to do partial writes, and implement an _atomic variant which does not take a spinlock. This API is used in xmon, so the less locking that is used, the better chance there is that a crash can be debugged.
Same comment I already had :-) "atomic" in Linux tends to mean something else (ie, atomic context), so I'd rather have something like opal_put_chars_sync() or such...

Oh yeah, I didn't ignore you, just... I thought atomic was okay. atomic *also* tends to mean happens atomically. I think the in atomic context meaning actually tends to be inatomic.

Sync I actually thought could be more easily confused with synchronous vs asynchronous here.

Quoted 140 lines

Cc Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h       |  1 +
 arch/powerpc/platforms/powernv/opal.c | 37 +++++++++++++++++++--------
 drivers/tty/hvc/hvc_opal.c            | 18 +++++++++----
 3 files changed, 41 insertions(+), 15 deletions(-)

arch/powerpc/include/asm/opal.h +1 -0

index bbff49fab0e5..5d7072411561 100644
@@ -303,6 +303,7 @@ extern void opal_configure_cores(void);
 
 extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
 extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
+extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
 extern int opal_flush_console(uint32_t vtermno);
 
 extern void hvc_opal_init_early(void);

arch/powerpc/platforms/powernv/opal.c +27 ~~-10~~

index 55d4b1983110..bcdb90ada938 100644
@@ -344,9 +344,9 @@ int opal_get_chars(uint32_t vtermno, char *buf, int count)
 	return 0;
 }
 
-int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+static int __opal_put_chars(uint32_t vtermno, const char *data, int total_len, bool atomic)
 {
-	unsigned long flags;
+	unsigned long flags = 0 /* shut up gcc */;
 	int written;
 	__be64 olen;
 	s64 rc;

@@ -354,11 +354,8 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	if (!opal.entry)
 		return -ENODEV;
 
-	/* We want put_chars to be atomic to avoid mangling of hvsi
-	 * packets. To do that, we first test for room and return
-	 * -EAGAIN if there isn't enough.
-	 */
-	spin_lock_irqsave(&opal_write_lock, flags);
+	if (atomic)
+		spin_lock_irqsave(&opal_write_lock, flags);
 	rc = opal_console_write_buffer_space(vtermno, &olen);
 	if (rc || be64_to_cpu(olen) < total_len) {
 		/* Closed -> drop characters */

@@ -391,14 +388,18 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 
 	written = be64_to_cpu(olen);
 	if (written < total_len) {
-		/* Should not happen */
-		pr_warn("atomic console write returned partial len=%d written=%d\n", total_len, written);
+		if (atomic) {
+			/* Should not happen */
+			pr_warn("atomic console write returned partial "
+				"len=%d written=%d\n", total_len, written);
+		}
 		if (!written)
 			written = -EAGAIN;
 	}
 
 out:
-	spin_unlock_irqrestore(&opal_write_lock, flags);
+	if (atomic)
+		spin_unlock_irqrestore(&opal_write_lock, flags);
 
 	/* In the -EAGAIN case, callers loop, so we have to flush the console
 	 * here in case they have interrupts off (and we don't want to wait

@@ -412,6 +413,22 @@ int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
 	return written;
 }
 
+int opal_put_chars(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, false);
+}
+
+/*
+ * opal_put_chars_atomic will not perform partial-writes. Data will be
+ * atomically written to the terminal or not at all. This is not strictly
+ * true at the moment because console space can race with OPAL's console
+ * writes.
+ */
+int opal_put_chars_atomic(uint32_t vtermno, const char *data, int total_len)
+{
+	return __opal_put_chars(vtermno, data, total_len, true);
+}
+
 int opal_flush_console(uint32_t vtermno)
 {
 	s64 rc;

drivers/tty/hvc/hvc_opal.c +13 -5

index af122ad7f06d..0a72f98ee082 100644
@@ -183,9 +183,15 @@ static int hvc_opal_probe(struct platform_device *dev)
 			return -ENOMEM;
 		pv->proto = proto;
 		hvc_opal_privs[termno] = pv;
-		if (proto == HV_PROTOCOL_HVSI)
-			hvsilib_init(&pv->hvsi, opal_get_chars, opal_put_chars,
+		if (proto == HV_PROTOCOL_HVSI) {
+			/*
+			 * We want put_chars to be atomic to avoid mangling of
+			 * hvsi packets.
+			 */
+			hvsilib_init(&pv->hvsi,
+				     opal_get_chars, opal_put_chars_atomic,
 				     termno, 0);
+		}
 
 		/* Instanciate now to establish a mapping index==vtermno */
 		hvc_instantiate(termno, termno, ops);

@@ -376,8 +382,9 @@ void __init hvc_opal_init_early(void)
 	else if (of_device_is_compatible(stdout_node,"ibm,opal-console-hvsi")) {
 		hvc_opal_boot_priv.proto = HV_PROTOCOL_HVSI;
 		ops = &hvc_opal_hvsi_ops;
-		hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars,
-			     opal_put_chars, index, 1);
+		hvsilib_init(&hvc_opal_boot_priv.hvsi,
+			     opal_get_chars, opal_put_chars_atomic,
+			     index, 1);
 		/* HVSI, perform the handshake now */
 		hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 		pr_devel("hvc_opal: Found HVSI console\n");

@@ -409,7 +416,8 @@ void __init udbg_init_debug_opal_hvsi(void)
 	hvc_opal_privs[index] = &hvc_opal_boot_priv;
 	hvc_opal_boot_termno = index;
 	udbg_init_opal_common();
-	hvsilib_init(&hvc_opal_boot_priv.hvsi, opal_get_chars, opal_put_chars,
+	hvsilib_init(&hvc_opal_boot_priv.hvsi,
+		     opal_get_chars, opal_put_chars_atomic,
 		     index, 1);
 	hvsilib_establish(&hvc_opal_boot_priv.hvsi);
 }

Michael Ellerman

reviewer

2018-05-07 10:35 UTC #16 lore raw

Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic

Quoted 14 lines · Nicholas Piggin

Quoted 10 lines · On Tue, 01 May 2018 19:48:58 +1000 Benjamin Herrenschmidt…
Quoted 5 lines · Nicholas Piggin
The RAW console does not need writes to be atomic, so relax opal_put_chars to be able to do partial writes, and implement an _atomic variant which does not take a spinlock. This API is used in xmon, so the less locking that is used, the better chance there is that a crash can be debugged.
Same comment I already had :-) "atomic" in Linux tends to mean something else (ie, atomic context), so I'd rather have something like opal_put_chars_sync() or such...

Quoted 6 lines

Oh yeah, I didn't ignore you, just... I thought atomic was okay. atomic *also* tends to mean happens atomically. I think the in atomic context meaning actually tends to be inatomic.
Sync I actually thought could be more easily confused with synchronous vs asynchronous here.

I think we probably want opal_put_chars() to stay as it is.

And then add a variant for the call (just xmon?) that want lock free behaviour.

opal_put_chars_unlocked() or something?

cheers

Nicholas Piggin

author reviewer

2018-05-08 03:36 UTC #17 lore raw

Re: [PATCH 08/15] powerpc/powernv: implement opal_put_chars_atomic

Quoted 20 lines · On Mon, 07 May 2018 20:35:42 +1000 Michael Ellerman <[email protected]> wrote:

Quoted 18 lines · Nicholas Piggin
Quoted 10 lines · On Tue, 01 May 2018 19:48:58 +1000 Benjamin Herrenschmidt…
Quoted 5 lines · Nicholas Piggin
The RAW console does not need writes to be atomic, so relax opal_put_chars to be able to do partial writes, and implement an _atomic variant which does not take a spinlock. This API is used in xmon, so the less locking that is used, the better chance there is that a crash can be debugged.
Same comment I already had :-) "atomic" in Linux tends to mean something else (ie, atomic context), so I'd rather have something like opal_put_chars_sync() or such...
Oh yeah, I didn't ignore you, just... I thought atomic was okay. atomic *also* tends to mean happens atomically. I think the in atomic context meaning actually tends to be inatomic.

Quoted 7 lines

Quoted 2 lines
Sync I actually thought could be more easily confused with synchronous vs asynchronous here.
I think we probably want opal_put_chars() to stay as it is.
And then add a variant for the call (just xmon?) that want lock free behaviour.

No it's not the lock which is important here, it is whether the message goes to the console atomically versus other writes. The raw console does not require this, only one which sends some control characters, which is the hvterm-protocol compatible variant of the vio console, and I think FSP console.

BMC consoles for example always use raw.

Quoted 1 line

opal_put_chars_unlocked() or something?

I prefer the _atomic as the special case. Ordinarily we don't have a special requirement, but with the control characters then we do.

Thanks,
Nick

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #18 lore raw

PATCH 9/15 tty: hvc: remove unexplained "just in case" spin delay

In reply to Nicholas Piggin

This delay was in the very first OPAL console commit 6.5 years ago, and came from the vio hvc driver. The firmware console has hardened sufficiently to remove it.

Reviewed-by Benjamin Herrenschmidt <[email protected]>

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_opal.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

drivers/tty/hvc/hvc_opal.c +1 -7

index 0a72f98ee082..786e76a1e06d 100644
@@ -313,14 +313,8 @@ static int udbg_opal_getc(void)
 	int ch;
 	for (;;) {
 		ch = udbg_opal_getc_poll();
-		if (ch == -1) {
-			/* This shouldn't be needed...but... */
-			volatile unsigned long delay;
-			for (delay=0; delay < 2000000; delay++)
-				;
-		} else {
+		if (ch != -1)
 			return ch;
-		}
 	}
 }

Signature · 1 line

2.17.0

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #19 lore raw

PATCH 10/15 tty: hvc: use mutex instead of spinlock for hvc_structs lock

In reply to Nicholas Piggin

This allows hvc operations to sleep under the lock.

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_console.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

drivers/tty/hvc/hvc_console.c +11 ~~-12~~

index 7709fcc707f4..fddb63322c67 100644
@@ -73,7 +73,7 @@ static LIST_HEAD(hvc_structs);
  * Protect the list of hvc_struct instances from inserts and removals during
  * list traversal.
  */
-static DEFINE_SPINLOCK(hvc_structs_lock);
+static DEFINE_MUTEX(hvc_structs_mutex);
 
 /*
  * This value is used to assign a tty->index value to a hvc_struct based

@@ -83,7 +83,7 @@ static DEFINE_SPINLOCK(hvc_structs_lock);
 static int last_hvc = -1;
 
 /*
- * Do not call this function with either the hvc_structs_lock or the hvc_struct
+ * Do not call this function with either the hvc_structs_mutex or the hvc_struct
  * lock held.  If successful, this function increments the kref reference
  * count against the target hvc_struct so it should be released when finished.
  */

@@ -92,25 +92,24 @@ static struct hvc_struct *hvc_get_by_index(int index)
 	struct hvc_struct *hp;
 	unsigned long flags;
 
-	spin_lock(&hvc_structs_lock);
+	mutex_lock(&hvc_structs_mutex);
 
 	list_for_each_entry(hp, &hvc_structs, next) {
 		spin_lock_irqsave(&hp->lock, flags);
 		if (hp->index == index) {
 			tty_port_get(&hp->port);
 			spin_unlock_irqrestore(&hp->lock, flags);
-			spin_unlock(&hvc_structs_lock);
+			mutex_unlock(&hvc_structs_mutex);
 			return hp;
 		}
 		spin_unlock_irqrestore(&hp->lock, flags);
 	}
 	hp = NULL;
+	mutex_unlock(&hvc_structs_mutex);
 
-	spin_unlock(&hvc_structs_lock);
 	return hp;
 }
 
-
 /*
  * Initial console vtermnos for console API usage prior to full console
  * initialization.  Any vty adapter outside this range will not have usable

@@ -224,13 +223,13 @@ static void hvc_port_destruct(struct tty_port *port)
 	struct hvc_struct *hp = container_of(port, struct hvc_struct, port);
 	unsigned long flags;
 
-	spin_lock(&hvc_structs_lock);
+	mutex_lock(&hvc_structs_mutex);
 
 	spin_lock_irqsave(&hp->lock, flags);
 	list_del(&(hp->next));
 	spin_unlock_irqrestore(&hp->lock, flags);
 
-	spin_unlock(&hvc_structs_lock);
+	mutex_unlock(&hvc_structs_mutex);
 
 	kfree(hp);
 }

@@ -733,11 +732,11 @@ static int khvcd(void *unused)
 		try_to_freeze();
 		wmb();
 		if (!cpus_are_in_xmon()) {
-			spin_lock(&hvc_structs_lock);
+			mutex_lock(&hvc_structs_mutex);
 			list_for_each_entry(hp, &hvc_structs, next) {
 				poll_mask |= hvc_poll(hp);
 			}
-			spin_unlock(&hvc_structs_lock);
+			mutex_unlock(&hvc_structs_mutex);
 		} else
 			poll_mask |= HVC_POLL_READ;
 		if (hvc_kicked)

@@ -871,7 +870,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 
 	INIT_WORK(&hp->tty_resize, hvc_set_winsz);
 	spin_lock_init(&hp->lock);
-	spin_lock(&hvc_structs_lock);
+	mutex_lock(&hvc_structs_mutex);
 
 	/*
 	 * find index to use:

@@ -891,7 +890,7 @@ struct hvc_struct *hvc_alloc(uint32_t vtermno, int data,
 	vtermnos[i] = vtermno;
 
 	list_add_tail(&(hp->next), &hvc_structs);
-	spin_unlock(&hvc_structs_lock);
+	mutex_unlock(&hvc_structs_mutex);
 
 	/* check if we need to re-register the kernel console */
 	hvc_check_console(i);

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-13 11:23 UTC #20 lore raw

Re: [10/15] tty: hvc: use mutex instead of spinlock for hvc_structs lock

Quoted 3 lines · Nicholas Piggin

This allows hvc operations to sleep under the lock.
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/a9bf5c8a271b9a954709b7ada1bd25

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #21 lore raw

PATCH 11/15 tty: hvc: hvc_poll break hv read loop

In reply to Nicholas Piggin

Avoid looping with the spinlock held while there is read data being returned from the hv driver. Instead note if the entire size returned by tty_buffer_request_room was read, and request another read poll.

This limits the critical section lengths, and provides more even service to other consoles in case there is a pathological condition.

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_console.c | 88 ++++++++++++++++++-----------------
 1 file changed, 45 insertions(+), 43 deletions(-)

drivers/tty/hvc/hvc_console.c +45 ~~-43~~

index fddb63322c67..745ac220fce8 100644
@@ -592,7 +592,7 @@ static u32 timeout = MIN_TIMEOUT;
 int hvc_poll(struct hvc_struct *hp)
 {
 	struct tty_struct *tty;
-	int i, n, poll_mask = 0;
+	int i, n, count, poll_mask = 0;
 	char buf[N_INBUF] __ALIGNED__;
 	unsigned long flags;
 	int read_total = 0;

@@ -618,7 +618,7 @@ int hvc_poll(struct hvc_struct *hp)
 
 	/* Now check if we can get data (are we throttled ?) */
 	if (tty_throttled(tty))
-		goto throttled;
+		goto out;
 
 	/* If we aren't notifier driven and aren't throttled, we always
 	 * request a reschedule

@@ -627,56 +627,58 @@ int hvc_poll(struct hvc_struct *hp)
 		poll_mask |= HVC_POLL_READ;
 
 	/* Read data if any */
-	for (;;) {
-		int count = tty_buffer_request_room(&hp->port, N_INBUF);
 
-		/* If flip is full, just reschedule a later read */
-		if (count == 0) {
+	count = tty_buffer_request_room(&hp->port, N_INBUF);
+
+	/* If flip is full, just reschedule a later read */
+	if (count == 0) {
+		poll_mask |= HVC_POLL_READ;
+		goto out;
+	}
+
+	n = hp->ops->get_chars(hp->vtermno, buf, count);
+	if (n <= 0) {
+		/* Hangup the tty when disconnected from host */
+		if (n == -EPIPE) {
+			spin_unlock_irqrestore(&hp->lock, flags);
+			tty_hangup(tty);
+			spin_lock_irqsave(&hp->lock, flags);
+		} else if ( n == -EAGAIN ) {
+			/*
+			 * Some back-ends can only ensure a certain min
+			 * num of bytes read, which may be > 'count'.
+			 * Let the tty clear the flip buff to make room.
+			 */
 			poll_mask |= HVC_POLL_READ;
-			break;
 		}
+		goto out;
+	}
 
-		n = hp->ops->get_chars(hp->vtermno, buf, count);
-		if (n <= 0) {
-			/* Hangup the tty when disconnected from host */
-			if (n == -EPIPE) {
-				spin_unlock_irqrestore(&hp->lock, flags);
-				tty_hangup(tty);
-				spin_lock_irqsave(&hp->lock, flags);
-			} else if ( n == -EAGAIN ) {
-				/*
-				 * Some back-ends can only ensure a certain min
-				 * num of bytes read, which may be > 'count'.
-				 * Let the tty clear the flip buff to make room.
-				 */
-				poll_mask |= HVC_POLL_READ;
-			}
-			break;
-		}
-		for (i = 0; i < n; ++i) {
+	for (i = 0; i < n; ++i) {
 #ifdef CONFIG_MAGIC_SYSRQ
-			if (hp->index == hvc_console.index) {
-				/* Handle the SysRq Hack */
-				/* XXX should support a sequence */
-				if (buf[i] == '\x0f') {	/* ^O */
-					/* if ^O is pressed again, reset
-					 * sysrq_pressed and flip ^O char */
-					sysrq_pressed = !sysrq_pressed;
-					if (sysrq_pressed)
-						continue;
-				} else if (sysrq_pressed) {
-					handle_sysrq(buf[i]);
-					sysrq_pressed = 0;
+		if (hp->index == hvc_console.index) {
+			/* Handle the SysRq Hack */
+			/* XXX should support a sequence */
+			if (buf[i] == '\x0f') {	/* ^O */
+				/* if ^O is pressed again, reset
+				 * sysrq_pressed and flip ^O char */
+				sysrq_pressed = !sysrq_pressed;
+				if (sysrq_pressed)
 					continue;
-				}
+			} else if (sysrq_pressed) {
+				handle_sysrq(buf[i]);
+				sysrq_pressed = 0;
+				continue;
 			}
-#endif /* CONFIG_MAGIC_SYSRQ */
-			tty_insert_flip_char(&hp->port, buf[i], 0);
 		}
-
-		read_total += n;
+#endif /* CONFIG_MAGIC_SYSRQ */
+		tty_insert_flip_char(&hp->port, buf[i], 0);
 	}
- throttled:
+	if (n == count)
+		poll_mask |= HVC_POLL_READ;
+	read_total = n;
+
+ out:
 	/* Wakeup write queue if necessary */
 	if (hp->do_wakeup) {
 		hp->do_wakeup = 0;

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-13 11:23 UTC #22 lore raw

Re: [11/15] tty: hvc: hvc_poll break hv read loop

Quoted 10 lines · Nicholas Piggin

Avoid looping with the spinlock held while there is read data being returned from the hv driver. Instead note if the entire size returned by tty_buffer_request_room was read, and request another read poll.
This limits the critical section lengths, and provides more even service to other consoles in case there is a pathological condition.
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/ec97eaad1383ab2500fcf9a07ade60

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #23 lore raw

PATCH 12/15 tty: hvc: hvc_poll may sleep

In reply to Nicholas Piggin

Introduce points where hvc_poll drops the lock, enables interrupts, and reschedules.

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_console.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

drivers/tty/hvc/hvc_console.c +14 -2

index 745ac220fce8..2abfc0b15fbb 100644
@@ -589,7 +589,7 @@ static u32 timeout = MIN_TIMEOUT;
 #define HVC_POLL_READ	0x00000001
 #define HVC_POLL_WRITE	0x00000002
 
-int hvc_poll(struct hvc_struct *hp)
+static int __hvc_poll(struct hvc_struct *hp, bool may_sleep)
 {
 	struct tty_struct *tty;
 	int i, n, count, poll_mask = 0;

@@ -611,6 +611,12 @@ int hvc_poll(struct hvc_struct *hp)
 		timeout = (written_total) ? 0 : MIN_TIMEOUT;
 	}
 
+	if (may_sleep) {
+		spin_unlock_irqrestore(&hp->lock, flags);
+		cond_resched();
+		spin_lock_irqsave(&hp->lock, flags);
+	}
+
 	/* No tty attached, just skip */
 	tty = tty_port_tty_get(&hp->port);
 	if (tty == NULL)

@@ -698,6 +704,11 @@ int hvc_poll(struct hvc_struct *hp)
 
 	return poll_mask;
 }
+
+int hvc_poll(struct hvc_struct *hp)
+{
+	return __hvc_poll(hp, false);
+}
 EXPORT_SYMBOL_GPL(hvc_poll);
 
 /**

@@ -736,7 +747,8 @@ static int khvcd(void *unused)
 		if (!cpus_are_in_xmon()) {
 			mutex_lock(&hvc_structs_mutex);
 			list_for_each_entry(hp, &hvc_structs, next) {
-				poll_mask |= hvc_poll(hp);
+				poll_mask |= __hvc_poll(hp, true);
+				cond_resched();
 			}
 			mutex_unlock(&hvc_structs_mutex);
 		} else

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-13 11:23 UTC #24 lore raw

Re: [12/15] tty: hvc: hvc_poll may sleep

Quoted 4 lines · Nicholas Piggin

Introduce points where hvc_poll drops the lock, enables interrupts, and reschedules.
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/cfb5946b55f1dfd19e042feae1fbff

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #25 lore raw

PATCH 13/15 tty: hvc: hvc_write may sleep

In reply to Nicholas Piggin

Rework the hvc_write loop to drop and re-take the spinlock on each iteration, add a cond_resched. Don't bother with an initial hvc_push initially, which makes the logic simpler -- just do a hvc_push on each time around the loop.

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_console.c | 36 ++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

drivers/tty/hvc/hvc_console.c +21 ~~-15~~

index 2abfc0b15fbb..6131d5084c42 100644
@@ -493,23 +493,29 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
 	if (hp->port.count <= 0)
 		return -EIO;
 
-	spin_lock_irqsave(&hp->lock, flags);
+	while (count > 0) {
+		spin_lock_irqsave(&hp->lock, flags);
 
-	/* Push pending writes */
-	if (hp->n_outbuf > 0)
-		hvc_push(hp);
-
-	while (count > 0 && (rsize = hp->outbuf_size - hp->n_outbuf) > 0) {
-		if (rsize > count)
-			rsize = count;
-		memcpy(hp->outbuf + hp->n_outbuf, buf, rsize);
-		count -= rsize;
-		buf += rsize;
-		hp->n_outbuf += rsize;
-		written += rsize;
-		hvc_push(hp);
+		rsize = hp->outbuf_size - hp->n_outbuf;
+
+		if (rsize) {
+			if (rsize > count)
+				rsize = count;
+			memcpy(hp->outbuf + hp->n_outbuf, buf, rsize);
+			count -= rsize;
+			buf += rsize;
+			hp->n_outbuf += rsize;
+			written += rsize;
+		}
+
+		if (hp->n_outbuf > 0)
+			hvc_push(hp);
+
+		spin_unlock_irqrestore(&hp->lock, flags);
+
+		if (count)
+			cond_resched();
 	}
-	spin_unlock_irqrestore(&hp->lock, flags);
 
 	/*
 	 * Racy, but harmless, kick thread if there is still pending data.

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-13 11:23 UTC #26 lore raw

Re: [13/15] tty: hvc: hvc_write may sleep

Quoted 6 lines · Nicholas Piggin

Rework the hvc_write loop to drop and re-take the spinlock on each iteration, add a cond_resched. Don't bother with an initial hvc_push initially, which makes the logic simpler -- just do a hvc_push on each time around the loop.
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/550ddadcc7580ec2a6c22d4ed04291

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #27 lore raw

PATCH 14/15 tty: hvc: introduce the hv_ops.flush operation for hvc drivers

In reply to Nicholas Piggin

Use .flush to wait for drivers to flush their console outside of the spinlock, to reduce lock/irq latencies.

Flush the hvc console driver after each write, which can help messages make it out to the console after a crash.

Signed-off-by Nicholas Piggin <[email protected]>

---
 drivers/tty/hvc/hvc_console.c | 35 +++++++++++++++++++++++++++++++++--
 drivers/tty/hvc/hvc_console.h |  1 +
 2 files changed, 34 insertions(+), 2 deletions(-)

drivers/tty/hvc/hvc_console.c +33 -2

index 6131d5084c42..5414c4a87bea 100644
@@ -110,6 +110,29 @@ static struct hvc_struct *hvc_get_by_index(int index)
 	return hp;
 }
 
+static int __hvc_flush(const struct hv_ops *ops, uint32_t vtermno, bool wait)
+{
+	if (wait)
+		might_sleep();
+
+	if (ops->flush)
+		return ops->flush(vtermno, wait);
+	return 0;
+}
+
+static int hvc_console_flush(const struct hv_ops *ops, uint32_t vtermno)
+{
+	return __hvc_flush(ops, vtermno, false);
+}
+
+/*
+ * Wait for the console to flush before writing more to it. This sleeps.
+ */
+static int hvc_flush(struct hvc_struct *hp)
+{
+	return __hvc_flush(hp->ops, hp->vtermno, true);
+}
+
 /*
  * Initial console vtermnos for console API usage prior to full console
  * initialization.  Any vty adapter outside this range will not have usable

@@ -155,8 +178,12 @@ static void hvc_console_print(struct console *co, const char *b,
 			if (r <= 0) {
 				/* throw away characters on error
 				 * but spin in case of -EAGAIN */
-				if (r != -EAGAIN)
+				if (r != -EAGAIN) {
 					i = 0;
+				} else {
+					hvc_console_flush(cons_ops[index],
+						      vtermnos[index]);
+				}
 			} else if (r > 0) {
 				i -= r;
 				if (i > 0)

@@ -164,6 +191,7 @@ static void hvc_console_print(struct console *co, const char *b,
 			}
 		}
 	}
+	hvc_console_flush(cons_ops[index], vtermnos[index]);
 }
 
 static struct tty_driver *hvc_console_device(struct console *c, int *index)

@@ -513,8 +541,11 @@ static int hvc_write(struct tty_struct *tty, const unsigned char *buf, int count
 
 		spin_unlock_irqrestore(&hp->lock, flags);
 
-		if (count)
+		if (count) {
+			if (hp->n_outbuf > 0)
+				hvc_flush(hp);
 			cond_resched();
+		}
 	}
 
 	/*

drivers/tty/hvc/hvc_console.h +1 -0

index ea63090e013f..e9319954c832 100644
@@ -54,6 +54,7 @@ struct hvc_struct {
 struct hv_ops {
 	int (*get_chars)(uint32_t vtermno, char *buf, int count);
 	int (*put_chars)(uint32_t vtermno, const char *buf, int count);
+	int (*flush)(uint32_t vtermno, bool wait);
 
 	/* Callbacks for notification. Called in open, close and hangup */
 	int (*notifier_add)(struct hvc_struct *hp, int irq);

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-13 11:23 UTC #28 lore raw

Re: [14/15] tty: hvc: introduce the hv_ops.flush operation for hvc drivers

Quoted 7 lines · Nicholas Piggin

Use .flush to wait for drivers to flush their console outside of the spinlock, to reduce lock/irq latencies.
Flush the hvc console driver after each write, which can help messages make it out to the console after a crash.
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/9f65b81f36e31563c5a5e4df3b3b8b

cheers

Nicholas Piggin

author reviewer

2018-04-30 14:55 UTC #29 lore raw

PATCH 15/15 powerpc/powernv: provide a console flush operation for opal hvc driver

In reply to Nicholas Piggin

Provide the flush hv_op for the opal hvc driver. This will flush the firmware console buffers without spinning with interrupts disabled.

Cc Benjamin Herrenschmidt <[email protected]>

Cc [email protected]

Signed-off-by Nicholas Piggin <[email protected]>

---
 arch/powerpc/include/asm/opal.h       |  1 +
 arch/powerpc/platforms/powernv/opal.c | 83 +++++++++++++++++----------
 drivers/tty/hvc/hvc_opal.c            |  2 +
 3 files changed, 55 insertions(+), 31 deletions(-)

arch/powerpc/include/asm/opal.h +1 -0

index 5d7072411561..a426d2caaf9f 100644
@@ -304,6 +304,7 @@ extern void opal_configure_cores(void);
 extern int opal_get_chars(uint32_t vtermno, char *buf, int count);
 extern int opal_put_chars(uint32_t vtermno, const char *buf, int total_len);
 extern int opal_put_chars_atomic(uint32_t vtermno, const char *buf, int total_len);
+extern int opal_flush_chars(uint32_t vtermno, bool wait);
 extern int opal_flush_console(uint32_t vtermno);
 
 extern void hvc_opal_init_early(void);

arch/powerpc/platforms/powernv/opal.c +52 ~~-31~~

119 lines not on this page · raw message

drivers/tty/hvc/hvc_opal.c +2 -0

index 786e76a1e06d..fd306033f8d4 100644
@@ -52,6 +52,7 @@ static u32 hvc_opal_boot_termno;
 static const struct hv_ops hvc_opal_raw_ops = {
 	.get_chars = opal_get_chars,
 	.put_chars = opal_put_chars,
+	.flush = opal_flush_chars,
 	.notifier_add = notifier_add_irq,
 	.notifier_del = notifier_del_irq,
 	.notifier_hangup = notifier_hangup_irq,

@@ -141,6 +142,7 @@ static int hvc_opal_hvsi_tiocmset(struct hvc_struct *hp, unsigned int set,
 static const struct hv_ops hvc_opal_hvsi_ops = {
 	.get_chars = hvc_opal_hvsi_get_chars,
 	.put_chars = hvc_opal_hvsi_put_chars,
+	.flush = opal_flush_chars,
 	.notifier_add = hvc_opal_hvsi_open,
 	.notifier_del = hvc_opal_hvsi_close,
 	.notifier_hangup = hvc_opal_hvsi_hangup,

Signature · 1 line

2.17.0

Michael Ellerman

Applied

2018-08-21 10:35 UTC #30 lore raw

Re: [15/15] powerpc/powernv: provide a console flush operation for opal hvc driver

Quoted 6 lines · Nicholas Piggin

Provide the flush hv_op for the opal hvc driver. This will flush the firmware console buffers without spinning with interrupts disabled.
Cc Benjamin Herrenschmidt <[email protected]>
Cc [email protected]
Signed-off-by Nicholas Piggin <[email protected]>

Applied to powerpc next, thanks.

https://git.kernel.org/powerpc/c/95b861a76c1ded3e89d33a3d9f4552

cheers