[Skiboot] [RFC PATCH 05/12] core/console: Add a seperate ABI memcons struct
Oliver O'Halloran
oohall at gmail.com
Tue May 19 15:46:26 AEST 2020
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 at 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 --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),
--
2.26.2
More information about the Skiboot
mailing list