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

[Joel Stanley]

On 31 May 2018 at 13:59, Joel Stanley <joel at 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 at 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
>