Messages in this thread | | | From | Kees Cook <> | Date | Tue, 12 Feb 2019 12:43:36 -0800 | Subject | Re: [PATCH] pstore: Add boot loader log messages support |
| |
On Fri, Feb 1, 2019 at 12:30 AM Yue Hu <huyue2@yulong.com> wrote: > > From bac8bbcd6081b967422dc82074a41098a0cf5180 Mon Sep 17 00:00:00 2001 > From: Yue Hu <huyue2@yulong.com> > Date: Tue, 29 Jan 2019 11:42:27 +0800 > Subject: [PATCH] pstore: Add boot loader log messages support > > Sometimes we hope to check boot loader log messages (e.g. Android > Verified Boot status) when kernel is coming up. Generally it does > depend on serial device, but it will be removed for the hardware > shipping to market by most of manufacturers. In that case better > solder and proper serial cable for different interface (e.g. Type-C > or microUSB) are needed. That is inconvenient and even wasting much > time on it. > > Therefore, let's add a logging support: PSTORE_TYPE_XBL. > > Signed-off-by: Yue Hu <huyue2@yulong.com> > --- > fs/pstore/Kconfig | 10 ++++++++ > fs/pstore/platform.c | 16 +++++++++++++ > fs/pstore/ram.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++-- > include/linux/pstore.h | 21 +++++++++++++---- > 4 files changed, 104 insertions(+), 7 deletions(-) > > diff --git a/fs/pstore/Kconfig b/fs/pstore/Kconfig > index 0d19d19..ef4a2dc 100644 > --- a/fs/pstore/Kconfig > +++ b/fs/pstore/Kconfig > @@ -137,6 +137,16 @@ config PSTORE_FTRACE > > If unsure, say N. > > +config PSTORE_XBL > + bool "Log bootloader messages" > + depends on PSTORE > + help > + When the option is enabled, pstore will log boot loader > + messages to /sys/fs/pstore/xbl-ramoops-[ID] after reboot. > + Boot loader needs to support log buffer reserved. > + > + If unsure, say N. > + > config PSTORE_RAM > tristate "Log panic/oops to a RAM buffer" > depends on PSTORE > diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c > index 0ca7657..7486e48 100644 > --- a/fs/pstore/platform.c > +++ b/fs/pstore/platform.c > @@ -66,6 +66,7 @@ > "mce", > "console", > "ftrace", > + "xbl", > "rtas", > "powerpc-ofw", > "powerpc-common", > @@ -532,6 +533,19 @@ static void pstore_register_console(void) {} > static void pstore_unregister_console(void) {} > #endif > > +#ifdef CONFIG_PSTORE_XBL > +static void pstore_register_xbl(void) > +{ > + struct pstore_record record; > + > + pstore_record_init(&record, psinfo); > + record.type = PSTORE_TYPE_XBL; > + psinfo->write(&record); > +}
This writes a zero-length record at registration time. Why?
> +#else > +static void pstore_register_xbl(void) {} > +#endif > + > static int pstore_write_user_compat(struct pstore_record *record, > const char __user *buf) > { > @@ -618,6 +632,8 @@ int pstore_register(struct pstore_info *psi) > pstore_register_ftrace(); > if (psi->flags & PSTORE_FLAGS_PMSG) > pstore_register_pmsg(); > + if (psi->flags & PSTORE_FLAGS_XBL) > + pstore_register_xbl(); > > /* Start watching for new records, if desired. */ > if (pstore_update_ms >= 0) { > diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c > index ca01778..0bb086b 100644 > --- a/fs/pstore/ram.c > +++ b/fs/pstore/ram.c > @@ -56,6 +56,10 @@ > module_param_named(pmsg_size, ramoops_pmsg_size, ulong, 0400); > MODULE_PARM_DESC(pmsg_size, "size of user space message log"); > > +static ulong ramoops_xbl_size = MIN_MEM_SIZE; > +module_param_named(xbl_size, ramoops_xbl_size, ulong, 0400); > +MODULE_PARM_DESC(xbl_size, "size of boot loader log"); > + > static unsigned long long mem_address; > module_param_hw(mem_address, ullong, other, 0400); > MODULE_PARM_DESC(mem_address, > @@ -88,6 +92,7 @@ struct ramoops_context { > struct persistent_ram_zone *cprz; /* Console zone */ > struct persistent_ram_zone **fprzs; /* Ftrace zones */ > struct persistent_ram_zone *mprz; /* PMSG zone */ > + struct persistent_ram_zone *bprz; /* XBL zone */ > phys_addr_t phys_addr; > unsigned long size; > unsigned int memtype; > @@ -95,6 +100,7 @@ struct ramoops_context { > size_t console_size; > size_t ftrace_size; > size_t pmsg_size; > + size_t xbl_size; > int dump_oops; > u32 flags; > struct persistent_ram_ecc_info ecc_info; > @@ -106,6 +112,7 @@ struct ramoops_context { > unsigned int max_ftrace_cnt; > unsigned int ftrace_read_cnt; > unsigned int pmsg_read_cnt; > + unsigned int xbl_read_cnt; > struct pstore_info pstore; > }; > > @@ -119,6 +126,7 @@ static int ramoops_pstore_open(struct pstore_info *psi) > cxt->console_read_cnt = 0; > cxt->ftrace_read_cnt = 0; > cxt->pmsg_read_cnt = 0; > + cxt->xbl_read_cnt = 0; > return 0; > } > > @@ -272,6 +280,10 @@ static ssize_t ramoops_pstore_read(struct pstore_record *record) > if (!prz_ok(prz) && !cxt->pmsg_read_cnt++) > prz = ramoops_get_next_prz(&cxt->mprz, 0 /* single */, record); > > + if (!prz_ok(prz) && !cxt->xbl_read_cnt++) { > + prz = ramoops_get_next_prz(&cxt->bprz, 0 /* single */, record); > + } > + > /* ftrace is last since it may want to dynamically allocate memory. */ > if (!prz_ok(prz)) { > if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) && > @@ -360,6 +372,26 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, > return len; > } > > +static void ramoops_get_xbl_record(struct persistent_ram_zone *prz, > + struct pstore_record *record, > + size_t xbl_size) > +{ > + struct pstore_xbl_header *hdr = NULL; > + size_t half = xbl_size / 2; > + size_t max = half - PSTORE_XBL_HDR_SIZE; > + > + hdr = (struct pstore_xbl_header *)((size_t)prz->buffer + half);
Before reading this data, please verify the prz buffer sizes (since prior boots may have had a different size, etc).
> + > + if (hdr->cookie == PSTORE_XBL_COOKIE) { > + record->buf = (char *)((size_t)hdr + PSTORE_XBL_HDR_SIZE); > + record->size = hdr->size_written; > + if (unlikely(record->size > max)) > + record->size = max; > + return; > + } > + pr_debug("no valid xbl record (cookie = 0x%08x)\n", hdr->cookie); > +} > + > static int notrace ramoops_pstore_write(struct pstore_record *record) > { > struct ramoops_context *cxt = record->psi->data; > @@ -390,6 +422,16 @@ static int notrace ramoops_pstore_write(struct pstore_record *record) > } else if (record->type == PSTORE_TYPE_PMSG) { > pr_warn_ratelimited("PMSG shouldn't call %s\n", __func__); > return -EINVAL; > + } else if (record->type == PSTORE_TYPE_XBL) { > + if (!cxt->bprz) > + return -ENOMEM; > + > + ramoops_get_xbl_record(cxt->bprz, record, cxt->xbl_size); > + if (record->size == 0) > + return -EINVAL; > + > + persistent_ram_write(cxt->bprz, record->buf, record->size); > + return 0; > } > > if (record->type != PSTORE_TYPE_DMESG) > @@ -469,6 +511,9 @@ static int ramoops_pstore_erase(struct pstore_record *record) > case PSTORE_TYPE_PMSG: > prz = cxt->mprz; > break; > + case PSTORE_TYPE_XBL: > + prz = cxt->bprz; > + break; > default: > return -EINVAL; > } > @@ -697,6 +742,7 @@ static int ramoops_parse_dt(struct platform_device *pdev, > parse_size("console-size", pdata->console_size); > parse_size("ftrace-size", pdata->ftrace_size); > parse_size("pmsg-size", pdata->pmsg_size); > + parse_size("xbl-size", pdata->xbl_size); > parse_size("ecc-size", pdata->ecc_info.ecc_size); > parse_size("flags", pdata->flags); > > @@ -740,7 +786,7 @@ static int ramoops_probe(struct platform_device *pdev) > } > > if (!pdata->mem_size || (!pdata->record_size && !pdata->console_size && > - !pdata->ftrace_size && !pdata->pmsg_size)) { > + !pdata->ftrace_size && !pdata->pmsg_size && !pdata->xbl_size)) { > pr_err("The memory size and the record/console size must be " > "non-zero\n"); > goto fail_out; > @@ -754,6 +800,8 @@ static int ramoops_probe(struct platform_device *pdev) > pdata->ftrace_size = rounddown_pow_of_two(pdata->ftrace_size); > if (pdata->pmsg_size && !is_power_of_2(pdata->pmsg_size)) > pdata->pmsg_size = rounddown_pow_of_two(pdata->pmsg_size); > + if (pdata->xbl_size && !is_power_of_2(pdata->xbl_size)) > + pdata->xbl_size = rounddown_pow_of_two(pdata->xbl_size); > > cxt->size = pdata->mem_size; > cxt->phys_addr = pdata->mem_address; > @@ -762,6 +810,7 @@ static int ramoops_probe(struct platform_device *pdev) > cxt->console_size = pdata->console_size; > cxt->ftrace_size = pdata->ftrace_size; > cxt->pmsg_size = pdata->pmsg_size; > + cxt->xbl_size = pdata->xbl_size; > cxt->dump_oops = pdata->dump_oops; > cxt->flags = pdata->flags; > cxt->ecc_info = pdata->ecc_info; > @@ -769,7 +818,7 @@ static int ramoops_probe(struct platform_device *pdev) > paddr = cxt->phys_addr; > > dump_mem_sz = cxt->size - cxt->console_size - cxt->ftrace_size > - - cxt->pmsg_size; > + - cxt->pmsg_size - cxt->xbl_size; > err = ramoops_init_przs("dmesg", dev, cxt, &cxt->dprzs, &paddr, > dump_mem_sz, cxt->record_size, > &cxt->max_dump_cnt, 0, 0); > @@ -797,6 +846,11 @@ static int ramoops_probe(struct platform_device *pdev) > if (err) > goto fail_init_mprz; > > + err = ramoops_init_prz("xbl", dev, cxt, &cxt->bprz, &paddr, > + cxt->xbl_size, 0); > + if (err) > + goto fail_init_bprz; > + > cxt->pstore.data = cxt; > /* > * Since bufsize is only used for dmesg crash dumps, it > @@ -818,6 +872,8 @@ static int ramoops_probe(struct platform_device *pdev) > cxt->pstore.flags |= PSTORE_FLAGS_FTRACE; > if (cxt->pmsg_size) > cxt->pstore.flags |= PSTORE_FLAGS_PMSG; > + if (cxt->xbl_size) > + cxt->pstore.flags |= PSTORE_FLAGS_XBL; > > err = pstore_register(&cxt->pstore); > if (err) { > @@ -835,6 +891,7 @@ static int ramoops_probe(struct platform_device *pdev) > dump_oops = pdata->dump_oops; > ramoops_console_size = pdata->console_size; > ramoops_pmsg_size = pdata->pmsg_size; > + ramoops_xbl_size = pdata->xbl_size; > ramoops_ftrace_size = pdata->ftrace_size; > > pr_info("using 0x%lx@0x%llx, ecc: %d\n", > @@ -847,6 +904,8 @@ static int ramoops_probe(struct platform_device *pdev) > kfree(cxt->pstore.buf); > fail_clear: > cxt->pstore.bufsize = 0; > + persistent_ram_free(cxt->bprz); > +fail_init_bprz: > persistent_ram_free(cxt->mprz); > fail_init_mprz: > fail_init_fprz: > @@ -915,6 +974,7 @@ static void __init ramoops_register_dummy(void) > pdata.console_size = ramoops_console_size; > pdata.ftrace_size = ramoops_ftrace_size; > pdata.pmsg_size = ramoops_pmsg_size; > + pdata.xbl_size = ramoops_xbl_size; > pdata.dump_oops = dump_oops; > pdata.flags = RAMOOPS_FLAG_FTRACE_PER_CPU; > > diff --git a/include/linux/pstore.h b/include/linux/pstore.h > index b146181..e016bbf 100644 > --- a/include/linux/pstore.h > +++ b/include/linux/pstore.h > @@ -43,13 +43,14 @@ enum pstore_type_id { > PSTORE_TYPE_MCE = 1, > PSTORE_TYPE_CONSOLE = 2, > PSTORE_TYPE_FTRACE = 3, > + PSTORE_TYPE_XBL = 4,
Instead of reordering, please just append after PPC_OPAL.
> /* PPC64-specific partition types */ > - PSTORE_TYPE_PPC_RTAS = 4, > - PSTORE_TYPE_PPC_OF = 5, > - PSTORE_TYPE_PPC_COMMON = 6, > - PSTORE_TYPE_PMSG = 7, > - PSTORE_TYPE_PPC_OPAL = 8, > + PSTORE_TYPE_PPC_RTAS = 5, > + PSTORE_TYPE_PPC_OF = 6, > + PSTORE_TYPE_PPC_COMMON = 7, > + PSTORE_TYPE_PMSG = 8, > + PSTORE_TYPE_PPC_OPAL = 9, > > /* End of the list */ > PSTORE_TYPE_MAX > @@ -207,10 +208,20 @@ struct pstore_info { > #define PSTORE_FLAGS_CONSOLE BIT(1) > #define PSTORE_FLAGS_FTRACE BIT(2) > #define PSTORE_FLAGS_PMSG BIT(3) > +#define PSTORE_FLAGS_XBL BIT(4) > > extern int pstore_register(struct pstore_info *); > extern void pstore_unregister(struct pstore_info *); > > +#define PSTORE_XBL_COOKIE 0x4c425850 /* "PXBL" in ASCII */ > + > +struct pstore_xbl_header { > + uint32_t cookie; > + uint32_t size_written; > +}; > + > +#define PSTORE_XBL_HDR_SIZE sizeof(struct pstore_xbl_header) > + > struct pstore_ftrace_record { > unsigned long ip; > unsigned long parent_ip;
I don't see anything that actually _writes_ the PXBL? How does pstore interact with the boot loader to get the logs?
-- Kees Cook
| |