diff mbox series

[RFC,05/12] core/console: Add a seperate ABI memcons struct

Message ID 20200519054633.113238-6-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
This patch renames the existing memcons structure to memcons_desc and
adds a new memcons structure that is used internally by skiboot. There's
two main reasons for doing this:

a) The memcons_desc may be DMA mapped as writable so the FSP/BMC can
   write to it to implement the "input" portion of the in-memory
   console. As a result we should not be blindling trusting the buffer
   pointers from the descriptor and we should validate the offsets are
   correct.

b) Skiboot can be running little endian so we to handle the endian
   conversions which can get a bit unwieldly.

Seperating the external desctriptor (which is ABI) and the internal
console state helps to address both issues. We can rely on the internal
state since it can never be tampered with externally, and we can use
the internal state to regenerate the external descriptor as-needed.

Signed-off-by: Oliver O'Halloran <oohall@gmail.com>
---
 core/console.c                   | 39 ++++++++++++++++++++++----------
 include/console.h                | 22 +++++++++++++++++-
 platforms/ibm-fsp/common.c       |  6 ++---
 platforms/ibm-fsp/hostservices.c |  2 +-
 4 files changed, 52 insertions(+), 17 deletions(-)
diff mbox series

Patch

diff --git a/core/console.c b/core/console.c
index a1ec687cb67e..8be7ee578b18 100644
--- a/core/console.c
+++ b/core/console.c
@@ -17,6 +17,12 @@ 
 
 static char *con_buf = (char *)INMEM_CON_START;
 static size_t con_in;
+
+/*
+ * Skiboot is both the producer and consumer of the memcons. On the consumer
+ * side we need to keep track of how much of the log buffer has been written
+ * out to the console which is what con_out is for.
+ */
 static size_t con_out;
 static bool con_wrapped;
 
@@ -29,7 +35,7 @@  static struct opal_con_ops *opal_con_driver = &dummy_opal_con;
 static struct lock con_lock = LOCK_UNLOCKED;
 
 /* This is mapped via TCEs so we keep it alone in a page */
-struct memcons memcons __section(".data.memcons") = {
+struct memcons_desc memcons_desc __section(".data.memcons") = {
 	.magic		= CPU_TO_BE64(MEMCONS_MAGIC),
 	.obuf_phys	= CPU_TO_BE64(INMEM_CON_START),
 	.ibuf_phys	= CPU_TO_BE64(INMEM_CON_START + INMEM_CON_OUT_LEN),
@@ -37,6 +43,15 @@  struct memcons memcons __section(".data.memcons") = {
 	.ibuf_size	= CPU_TO_BE32(INMEM_CON_IN_LEN),
 };
 
+struct memcons memcons = {
+	.desc = &memcons_desc,
+	.obuf      = (char *) INMEM_CON_START,
+	.obuf_size =          INMEM_CON_OUT_LEN,
+
+	.ibuf      = (char *)  (INMEM_CON_START + INMEM_CON_OUT_LEN),
+	.ibuf_size =            INMEM_CON_IN_LEN,
+};
+
 static bool dummy_console_enabled(void)
 {
 #ifdef FORCE_DUMMY_CONSOLE
@@ -187,7 +202,7 @@  static void inmem_write(char c)
 	}
 
 	/*
-	 * We must always re-generate memcons.out_pos because
+	 * We must always re-generate memcons.desc->out_pos because
 	 * under some circumstances, the console script will
 	 * use a broken putmemproc that does RMW on the full
 	 * 8 bytes containing out_pos and in_prod, thus corrupting
@@ -197,7 +212,7 @@  static void inmem_write(char c)
 	if (con_wrapped)
 		opos |= MEMCONS_OUT_POS_WRAP;
 	lwsync();
-	memcons.out_pos = cpu_to_be32(opos);
+	memcons.desc->out_pos = cpu_to_be32(opos);
 }
 
 static void write_char(char c)
@@ -205,8 +220,8 @@  static void write_char(char c)
 	inmem_write(c);
 
 	/* If head reaches tail, push tail around & drop chars */
-	if (con_out == memcons.out_pos)
-		con_out = (memcons.out_pos + 1) % memcons.obuf_size;
+	if (con_out == memcons.desc->out_pos)
+		con_out = (memcons.desc->out_pos + 1) % memcons.desc->obuf_size;
 }
 
 ssize_t console_write(bool flush_to_drivers, const void *buf, size_t count)
@@ -240,12 +255,12 @@  ssize_t write(int fd __unused, const void *buf, size_t count)
 static size_t inmem_read(char *buf, size_t req)
 {
 	size_t read = 0;
-	char *ibuf = (char *)be64_to_cpu(memcons.ibuf_phys);
+	char *ibuf = (char *)be64_to_cpu(memcons.desc->ibuf_phys);
 
-	while (req && be32_to_cpu(memcons.in_prod) != be32_to_cpu(memcons.in_cons)) {
-		*(buf++) = ibuf[be32_to_cpu(memcons.in_cons)];
+	while (req && be32_to_cpu(memcons.desc->in_prod) != be32_to_cpu(memcons.desc->in_cons)) {
+		*(buf++) = ibuf[be32_to_cpu(memcons.desc->in_cons)];
 		lwsync();
-		memcons.in_cons = cpu_to_be32((be32_to_cpu(memcons.in_cons) + 1) % INMEM_CON_IN_LEN);
+		memcons.desc->in_cons = cpu_to_be32((be32_to_cpu(memcons.desc->in_cons) + 1) % INMEM_CON_IN_LEN);
 		req--;
 		read++;
 	}
@@ -335,7 +350,7 @@  void init_opal_console(void)
 
 void memcons_add_properties(void)
 {
-	dt_add_property_u64(opal_node, "ibm,opal-memcons", (u64) &memcons);
+	dt_add_property_u64(opal_node, "ibm,opal-memcons", (u64) &memcons_desc);
 }
 
 /*
@@ -407,7 +422,7 @@  static void dummy_console_poll(void *data __unused)
 	bool has_data = false;
 
 	lock(&con_lock);
-	if (memcons.in_prod != memcons.in_cons)
+	if (memcons.desc->in_prod != memcons.desc->in_cons)
 		has_data = true;
 	if (has_data)
 		opal_update_pending_evt(OPAL_EVENT_CONSOLE_INPUT,
@@ -421,7 +436,7 @@  void dummy_console_add_nodes(void)
 {
 	struct dt_property *p;
 
-	add_opal_console_node(0, "raw", be32_to_cpu(memcons.obuf_size));
+	add_opal_console_node(0, "raw", be32_to_cpu(memcons.desc->obuf_size));
 
 	/* Mambo might have left a crap one, clear it */
 	p = __dt_find_property(dt_chosen, "linux,stdout-path");
diff --git a/include/console.h b/include/console.h
index 6abd04b77d78..280f920aba44 100644
--- a/include/console.h
+++ b/include/console.h
@@ -12,21 +12,41 @@ 
  * facility or FSP.
  *
  * (This is v3 of the format, the previous one sucked)
+ *
+ * NB: This might be writable from the BMC! Don't blindly trust the contents.
  */
-struct memcons {
+struct memcons_desc {
 	__be64 magic;
 #define MEMCONS_MAGIC	0x6630696567726173LL
 	__be64 obuf_phys;
 	__be64 ibuf_phys;
 	__be32 obuf_size;
 	__be32 ibuf_size;
+
+	/* Output ring buffer write head. OPAL is the producer */
 	__be32 out_pos;
 #define MEMCONS_OUT_POS_WRAP	0x80000000u
 #define MEMCONS_OUT_POS_MASK	0x00ffffffu
+
+	/* Input ring buffer offsets. OPAL is the consumer */
 	__be32 in_prod;
 	__be32 in_cons;
 };
 
+/* internal version of the above, trustworthy, native endian, etc */
+struct memcons {
+	struct memcons_desc *desc;
+
+	char *obuf;
+	uint32_t obuf_size;
+
+	char *ibuf;
+	uint32_t ibuf_size;
+
+	uint32_t out_pos;
+	bool has_wrapped;
+};
+
 extern struct memcons memcons;
 
 #define INMEM_CON_IN_LEN	16
diff --git a/platforms/ibm-fsp/common.c b/platforms/ibm-fsp/common.c
index 4a723b25b064..6a20dc7ac36a 100644
--- a/platforms/ibm-fsp/common.c
+++ b/platforms/ibm-fsp/common.c
@@ -21,13 +21,13 @@  static void map_debug_areas(void)
 	/* Our memcons is in a section of its own and already
 	 * aligned to 4K. The buffers are mapped as a whole
 	 */
-	fsp_tce_map(PSI_DMA_MEMCONS, &memcons, 0x1000);
+	fsp_tce_map(PSI_DMA_MEMCONS, memcons.desc, 0x1000);
 	fsp_tce_map(PSI_DMA_LOG_BUF, (void*)INMEM_CON_START, INMEM_CON_LEN);
 
 	debug_descriptor.memcons_tce = cpu_to_be32(PSI_DMA_MEMCONS);
-	t = be64_to_cpu(memcons.obuf_phys) - INMEM_CON_START + PSI_DMA_LOG_BUF;
+	t = be64_to_cpu(memcons.desc->obuf_phys) - INMEM_CON_START + PSI_DMA_LOG_BUF;
 	debug_descriptor.memcons_obuf_tce = cpu_to_be32(t);
-	t = be64_to_cpu(memcons.ibuf_phys) - INMEM_CON_START + PSI_DMA_LOG_BUF;
+	t = be64_to_cpu(memcons.desc->ibuf_phys) - INMEM_CON_START + PSI_DMA_LOG_BUF;
 	debug_descriptor.memcons_ibuf_tce = cpu_to_be32(t);
 
 	t = PSI_DMA_TRACE_BASE;
diff --git a/platforms/ibm-fsp/hostservices.c b/platforms/ibm-fsp/hostservices.c
index 81fd6bdd3695..c0effc3e4067 100644
--- a/platforms/ibm-fsp/hostservices.c
+++ b/platforms/ibm-fsp/hostservices.c
@@ -177,7 +177,7 @@  static bool hbrt_con_wrapped;
 #define HBRT_CON_IN_LEN		0
 #define HBRT_CON_OUT_LEN	(HBRT_CON_LEN - HBRT_CON_IN_LEN)
 
-static struct memcons hbrt_memcons __section(".data.memcons") = {
+static struct memcons_desc hbrt_memcons __section(".data.memcons") = {
 	.magic		= CPU_TO_BE64(MEMCONS_MAGIC),
 	.obuf_phys	= CPU_TO_BE64(HBRT_CON_START),
 	.ibuf_phys	= CPU_TO_BE64(HBRT_CON_START + HBRT_CON_OUT_LEN),