[v3,3/3] dump: Always store kernel log buffer location

Message ID 20180531042943.15804-4-joel@jms.id.au
State New
Headers show
Series
  • BMC dumping
Related show

Commit Message

Joel Stanley May 31, 2018, 4:29 a.m.
When the OPAL_REGISTER_DUMP_REGION call is made to register the kernel's
log buffer, store that address in the debug descriptor. This address will
be used by eg. pdbg to fetch the host kernel's log buffer over FSI.

We unconditionally clear the value on a reboot (both fast-reboot and
reliable reboot) to reduce the chance of debug tools getting the wrong
buffer.

Signed-off-by: Joel Stanley <joel@jms.id.au>
---
 core/dump-region.c | 14 ++++++++++++--
 core/init.c        |  4 ++++
 2 files changed, 16 insertions(+), 2 deletions(-)

Comments

Joel Stanley May 31, 2018, 5:21 a.m. | #1
On 31 May 2018 at 13:59, Joel Stanley <joel@jms.id.au> wrote:
> When the OPAL_REGISTER_DUMP_REGION call is made to register the kernel's
> log buffer, store that address in the debug descriptor. This address will
> be used by eg. pdbg to fetch the host kernel's log buffer over FSI.
>
> We unconditionally clear the value on a reboot (both fast-reboot and
> reliable reboot) to reduce the chance of debug tools getting the wrong
> buffer.
>
> Signed-off-by: Joel Stanley <joel@jms.id.au>
> ---
>  core/dump-region.c | 14 ++++++++++++--
>  core/init.c        |  4 ++++
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/core/dump-region.c b/core/dump-region.c
> index afa4c2685b89..2917276a4097 100644
> --- a/core/dump-region.c
> +++ b/core/dump-region.c
> @@ -22,18 +22,28 @@
>  static int64_t opal_register_dump_region(uint32_t id, uint64_t addr,
>                                          uint64_t size)
>  {
> +       if (id == OPAL_DUMP_REGION_LOG_BUF) {
> +               prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr);
> +               debug_descriptor.log_buf_phys = addr;
> +       }
> +
>         if (platform.register_dump_region)
>                 return platform.register_dump_region(id, addr, size);
>
> -       return OPAL_UNSUPPORTED;
> +       return OPAL_SUCCESS;
>  }
>  opal_call(OPAL_REGISTER_DUMP_REGION, opal_register_dump_region, 3);
>
>  static int64_t opal_unregister_dump_region(uint32_t id)
>  {
> +       if (id == OPAL_DUMP_REGION_LOG_BUF) {
> +               prlog(PR_INFO, "Unregistered log buf\n");
> +               debug_descriptor.log_buf_phys = 0;
> +       }
> +
>         if (platform.unregister_dump_region)
>                 return platform.unregister_dump_region(id);
>
> -       return OPAL_UNSUPPORTED;
> +       return OPAL_SUCCESS;
>  }
>  opal_call(OPAL_UNREGISTER_DUMP_REGION, opal_unregister_dump_region, 1);
> diff --git a/core/init.c b/core/init.c
> index 3b887a24d11c..6caf0ba6913d 100644
> --- a/core/init.c
> +++ b/core/init.c
> @@ -563,6 +563,10 @@ void __noreturn load_and_boot_kernel(bool is_reboot)
>
>         mem_dump_free();
>
> +       /* Zero out memory location before the next kernel starts,
> +        * in case the previous kernel did not unregister */
> +       debug_descriptor.log_buf_phys = 0;

Mikey suggested we do something like this here:

 if (debug_descriptor.log_buf_phys) {
     pr_log(PR_INFO,
                "Previous kernel crashed? Clearing log buf address
%016"PRIx64"\n");
      debug_descriptor.log_buf_phys = 0;
 }

On a fast reboot there's a chance the data will hang around for a bit.

What are the chances that it will be useful? Is it worth it?

Cheers,

Joel

> +
>         /* Take processours out of nap */
>         cpu_set_sreset_enable(false);
>         cpu_set_ipi_enable(false);
> --
> 2.17.0
>

Patch

diff --git a/core/dump-region.c b/core/dump-region.c
index afa4c2685b89..2917276a4097 100644
--- a/core/dump-region.c
+++ b/core/dump-region.c
@@ -22,18 +22,28 @@ 
 static int64_t opal_register_dump_region(uint32_t id, uint64_t addr,
 					 uint64_t size)
 {
+	if (id == OPAL_DUMP_REGION_LOG_BUF) {
+		prlog(PR_INFO, "Registered log buf at 0x%016llx\n", addr);
+		debug_descriptor.log_buf_phys = addr;
+	}
+
 	if (platform.register_dump_region)
 		return platform.register_dump_region(id, addr, size);
 
-	return OPAL_UNSUPPORTED;
+	return OPAL_SUCCESS;
 }
 opal_call(OPAL_REGISTER_DUMP_REGION, opal_register_dump_region, 3);
 
 static int64_t opal_unregister_dump_region(uint32_t id)
 {
+	if (id == OPAL_DUMP_REGION_LOG_BUF) {
+		prlog(PR_INFO, "Unregistered log buf\n");
+		debug_descriptor.log_buf_phys = 0;
+	}
+
 	if (platform.unregister_dump_region)
 		return platform.unregister_dump_region(id);
 
-	return OPAL_UNSUPPORTED;
+	return OPAL_SUCCESS;
 }
 opal_call(OPAL_UNREGISTER_DUMP_REGION, opal_unregister_dump_region, 1);
diff --git a/core/init.c b/core/init.c
index 3b887a24d11c..6caf0ba6913d 100644
--- a/core/init.c
+++ b/core/init.c
@@ -563,6 +563,10 @@  void __noreturn load_and_boot_kernel(bool is_reboot)
 
 	mem_dump_free();
 
+	/* Zero out memory location before the next kernel starts,
+	 * in case the previous kernel did not unregister */
+	debug_descriptor.log_buf_phys = 0;
+
 	/* Take processours out of nap */
 	cpu_set_sreset_enable(false);
 	cpu_set_ipi_enable(false);