[Skiboot] [PATCH v2] nvram: Fix 'missing' nvram on FSP systems.
Cyril Bur
cyril.bur at au1.ibm.com
Fri Dec 1 14:53:07 AEDT 2017
commit ba4d46fdd9eb ("console: Set log level from nvram") wants to read
from NVRAM rather early. This works fine on BMC based systems as
nvram_init() is actually synchronous. This is not true for FSP systems
and it turns out that the query for the console log level simply
queries blank nvram.
The simple fix is to wait for the NVRAM read to complete before
performing any query. Unfortunately it turns out that the fsp-nvram
code does not inform the generic NVRAM layer when the read is complete,
rather, it must be prompted to do so.
This patch addresses both these problems. This patch adds a check before
the first read of the NVRAM (for the console log level) that the read
has completed. The fsp-nvram code has been updated to inform the generic
layer as soon as the read completes.
The old prompt to the fsp-nvram code has been removed but a check to
ensure that the NVRAM has been loaded remains. It is conservative but
if the NVRAM is not done loading before the host is booted it will not
have an nvram device-tree node which means it won't be able to access
the NVRAM at all, ever, even after the NVRAM has loaded.
Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
core/init.c | 2 +-
core/nvram-format.c | 9 +++++++++
core/nvram.c | 35 +++++++++++++++++++++++++++++++++++
core/test/run-nvram-format.c | 10 ++++++++++
hw/fsp/fsp-nvram.c | 28 ++++------------------------
include/fsp.h | 1 -
include/nvram.h | 2 ++
7 files changed, 61 insertions(+), 26 deletions(-)
diff --git a/core/init.c b/core/init.c
index 8951e17b..8323e2af 100644
--- a/core/init.c
+++ b/core/init.c
@@ -497,7 +497,7 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
/* We wait for the nvram read to complete here so we can
* grab stuff from there such as the kernel arguments
*/
- fsp_nvram_wait_open();
+ nvram_wait_for_load();
/* Wait for FW VPD data read to complete */
fsp_code_update_wait_vpd(true);
diff --git a/core/nvram-format.c b/core/nvram-format.c
index 923098af..3d030a30 100644
--- a/core/nvram-format.c
+++ b/core/nvram-format.c
@@ -216,6 +216,15 @@ const char *nvram_query(const char *key)
const char *part_end, *start;
int key_len = strlen(key);
+ if (!nvram_has_loaded()) {
+ prlog(PR_WARNING, "NVRAM: Query before is done loading\n");
+ prlog(PR_WARNING, "NVRAM: Waiting for load\n");
+ if (!nvram_wait_for_load()) {
+ prlog(PR_CRIT, "NVRAM: Failed to load\n");
+ return NULL;
+ }
+ }
+
/*
* The running OS can modify the NVRAM as it pleases so we need to be
* a little paranoid and check that it's ok before we try parse it.
diff --git a/core/nvram.c b/core/nvram.c
index 2140706a..de6cbddf 100644
--- a/core/nvram.c
+++ b/core/nvram.c
@@ -122,6 +122,41 @@ void nvram_read_complete(bool success)
nvram_ready = true;
}
+bool nvram_wait_for_load(void)
+{
+ /* Short cut */
+ if (nvram_ready)
+ return true;
+
+ /* Tell the caller it will never happen */
+ if (!platform.nvram_info)
+ return false;
+
+ /*
+ * One of two things has happened here.
+ * 1. nvram_wait_for_load() was called before nvram_init()
+ * 2. The read of NVRAM failed.
+ * Either way, this is quite a bad event.
+ */
+ if (!nvram_image && !nvram_size) {
+ prlog(PR_CRIT, "NVRAM: Possible wait before nvram_init()!\n");
+ return false;
+ }
+
+ while (!nvram_ready) {
+ opal_run_pollers();
+ /* If the read fails, tell the caller */
+ if (!nvram_image && !nvram_size)
+ return false;
+ }
+ return true;
+}
+
+bool nvram_has_loaded(void)
+{
+ return nvram_ready;
+}
+
void nvram_init(void)
{
int rc;
diff --git a/core/test/run-nvram-format.c b/core/test/run-nvram-format.c
index 5bd8ea2a..d86b1dcf 100644
--- a/core/test/run-nvram-format.c
+++ b/core/test/run-nvram-format.c
@@ -18,11 +18,21 @@
#include "../nvram-format.c"
+bool nvram_wait_for_load(void)
+{
+ return true;
+}
+
bool nvram_validate(void)
{
return true;
}
+bool nvram_has_loaded(void)
+{
+ return true;
+}
+
static char *nvram_reset(void *nvram_image, int size)
{
struct chrp_nvram_hdr *h = nvram_image;
diff --git a/hw/fsp/fsp-nvram.c b/hw/fsp/fsp-nvram.c
index 85b7e815..52342fcf 100644
--- a/hw/fsp/fsp-nvram.c
+++ b/hw/fsp/fsp-nvram.c
@@ -203,6 +203,10 @@ static void fsp_nvram_rd_complete(struct fsp_msg *msg)
*/
}
unlock(&fsp_nvram_lock);
+ nvram_read_complete(fsp_nvram_state == NVRAM_STATE_OPEN);
+ if (fsp_nvram_state != NVRAM_STATE_OPEN)
+ log_simple_error(&e_info(OPAL_RC_NVRAM_INIT),
+ "FSP: NVRAM not read, skipping init\n");
}
static void fsp_nvram_send_read(void)
@@ -387,27 +391,3 @@ int fsp_nvram_write(uint32_t offset, void *src, uint32_t size)
return 0;
}
-
-/* This is called right before starting the payload (Linux) to
- * ensure the initial open & read of nvram has happened before
- * we transfer control as the guest OS. This is necessary as
- * Linux will not handle a OPAL_BUSY return properly and treat
- * it as an error
- */
-void fsp_nvram_wait_open(void)
-{
- if (!fsp_present())
- return;
-
- while(fsp_nvram_state == NVRAM_STATE_OPENING)
- opal_run_pollers();
-
- if (!fsp_nvram_was_read) {
- log_simple_error(&e_info(OPAL_RC_NVRAM_INIT),
- "FSP: NVRAM not read, skipping init\n");
- nvram_read_complete(false);
- return;
- }
-
- nvram_read_complete(true);
-}
diff --git a/include/fsp.h b/include/fsp.h
index 1d877d81..e4bcae02 100644
--- a/include/fsp.h
+++ b/include/fsp.h
@@ -776,7 +776,6 @@ extern void fsp_used_by_console(void);
extern int fsp_nvram_info(uint32_t *total_size);
extern int fsp_nvram_start_read(void *dst, uint32_t src, uint32_t len);
extern int fsp_nvram_write(uint32_t offset, void *src, uint32_t size);
-extern void fsp_nvram_wait_open(void);
/* RTC */
extern void fsp_rtc_init(void);
diff --git a/include/nvram.h b/include/nvram.h
index 288b5368..012c107f 100644
--- a/include/nvram.h
+++ b/include/nvram.h
@@ -21,6 +21,8 @@ int nvram_format(void *nvram_image, uint32_t nvram_size);
int nvram_check(void *nvram_image, uint32_t nvram_size);
void nvram_reinit(void);
bool nvram_validate(void);
+bool nvram_has_loaded(void);
+bool nvram_wait_for_load(void);
const char *nvram_query(const char *name);
bool nvram_query_eq(const char *key, const char *value);
--
2.15.1
More information about the Skiboot
mailing list