diff mbox series

[RFC,12/12] core/console: Add a seperate lock for the in-memory console

Message ID 20200519054633.113238-13-oohall@gmail.com
State RFC
Headers show
Series [RFC,01/12] platform/mambo: Add a mambo OPAL console driver | expand

Checks

Context Check Description
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot-dco success Signed-off-by present
snowpatch_ozlabs/snowpatch_job_snowpatch-skiboot success Test snowpatch/job/snowpatch-skiboot on branch master
snowpatch_ozlabs/apply_patch success Successfully applied on branch master (0f1937ef40fca0c3212a9dff1010b832a24fb063)

Commit Message

Oliver O'Halloran May 19, 2020, 5:46 a.m. UTC
With the msglog split from the in-mem console we shouldn't be using the
same lock for both. Add a seperate dummy_con_lock() which is used for
the in-memory console driver.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---

Renaming con_lock to msglog_lock would probably make sense.
Folding this into the previous patch might also make sense.
---
 core/console.c | 64 ++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 46 insertions(+), 18 deletions(-)
diff mbox series

Patch

diff --git a/core/console.c b/core/console.c
index bfeb7aae39e0..226ed57f0d36 100644
--- a/core/console.c
+++ b/core/console.c
@@ -29,6 +29,7 @@  static struct con_ops *con_driver;
 static struct opal_con_ops *opal_con_driver = &dummy_opal_con;
 
 static struct lock con_lock = LOCK_UNLOCKED;
+static struct lock dummy_con_lock = LOCK_UNLOCKED;
 
 /* The descriptors are is mapped via TCEs so we keep it alone in a page */
 struct memcons_desc msglog_desc __section(".data.memcons") = {
@@ -264,7 +265,17 @@  ssize_t write(int fd __unused, const void *buf, size_t count)
 /* flushes the msglog into the inmem_con */
 static size_t msglog_glue(const char *buf, size_t len)
 {
-	return write(0, buf, len);
+	int i;
+
+	lock(&dummy_con_lock);
+	for (i = 0; i < len; i++)
+		inmem_write(&inmem_con, buf[i]);
+#ifdef MAMBO_DEBUG_CONSOLE
+	mambo_console_write(buf, len);
+#endif
+	unlock(&dummy_con_lock);
+
+	return len;
 }
 
 static struct con_ops dummy_con_glue = {
@@ -286,17 +297,20 @@  static size_t inmem_read(struct memcons *mcon, char *buf, size_t req)
 	return read;
 }
 
-ssize_t read(int fd __unused, void *buf, size_t req_count)
+ssize_t __noreturn read(int fd, void *buf, size_t req_count)
 {
-	bool need_unlock = lock_recursive(&con_lock);
-	size_t count = 0;
+	/*
+	 * We keep write() around because our libc stdio functions need it, and
+	 * there's a few bits of library code that call fprintf and friends.
+	 *
+	 * There's no real use case for read() inside skiboot so we should
+	 * never hit this path.
+	 */
+	(void) fd;
+	(void) buf;
+	(void) req_count;
 
-	/* NB: we are reading the input buffer of inmem_con, *not* msglog */
-	if (!count)
-		count = inmem_read(&inmem_con, buf, req_count);
-	if (need_unlock)
-		unlock(&con_lock);
-	return count;
+	abort();
 }
 
 /* Helper function to perform a full synchronous flush */
@@ -385,14 +399,13 @@  void msglog_add_properties(void)
  * The default OPAL console.
  *
  * In the absence of a "real" OPAL console driver we handle the OPAL_CONSOLE_*
- * calls by writing into the skiboot log buffer. Reads are a little more
- * complicated since they can come from the in-memory console (BML) or from the
- * internal skiboot console driver.
+ * calls by writing into the in-memory console. Since this is entirely in-memory
+ * some external tooling is required to actually read the console.
  */
 static int64_t dummy_console_write(int64_t term_number, __be64 *length,
 				   const uint8_t *buffer)
 {
-	uint64_t l;
+	uint64_t i, l;
 
 	if (term_number != 0)
 		return OPAL_PARAMETER;
@@ -401,7 +414,17 @@  static int64_t dummy_console_write(int64_t term_number, __be64 *length,
 		return OPAL_PARAMETER;
 
 	l = be64_to_cpu(*length);
-	write(0, buffer, l);
+
+	lock(&dummy_con_lock);
+
+	for (i = 0; i < l; i++)
+		inmem_write(&inmem_con, buffer[i]);
+#ifdef MAMBO_DEBUG_CONSOLE
+	mambo_console_write(buffer, l);
+#endif
+
+	/* updating l isn't required since we used all of the buffer */
+	unlock(&dummy_con_lock);
 
 	return OPAL_SUCCESS;
 }
@@ -432,11 +455,16 @@  static int64_t dummy_console_read(int64_t term_number, __be64 *length,
 	if (!opal_addr_valid(length) || !opal_addr_valid(buffer))
 		return OPAL_PARAMETER;
 
+	lock(&dummy_con_lock);
+
 	l = be64_to_cpu(*length);
-	l = read(0, buffer, l);
+	l = inmem_read(&inmem_con, buffer, l);
+
 	*length = cpu_to_be64(l);
 	opal_update_pending_evt(OPAL_EVENT_CONSOLE_INPUT, 0);
 
+	unlock(&dummy_con_lock);
+
 	return OPAL_SUCCESS;
 }
 
@@ -449,7 +477,7 @@  static void dummy_console_poll(void *data __unused)
 {
 	bool has_data = false;
 
-	lock(&con_lock);
+	lock(&dummy_con_lock);
 	if (inmem_con.desc->in_prod != inmem_con.desc->in_cons)
 		has_data = true;
 	if (has_data)
@@ -457,7 +485,7 @@  static void dummy_console_poll(void *data __unused)
 					OPAL_EVENT_CONSOLE_INPUT);
 	else
 		opal_update_pending_evt(OPAL_EVENT_CONSOLE_INPUT, 0);
-	unlock(&con_lock);
+	unlock(&dummy_con_lock);
 }
 
 void dummy_console_add_nodes(void)