| Message ID | 1676978713-7394-7-git-send-email-quic_mojha@quicinc.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add basic Minidump kernel driver support | expand |
Thanks Kees for checking into this. On 2/24/2023 1:13 AM, Kees Cook wrote: > On Tue, Feb 21, 2023 at 04:55:13PM +0530, Mukesh Ojha wrote: >> There are system which does not uses pstore directly but >> may have the interest in the context saved by pstore. >> Register pstore regions with minidump so that it get >> dumped on minidump collection. > > Okay, so, this is a really interesting case -- it's a RAM backend that > is already found on a system by pstore via device tree, but there is > _another_ RAM overlay (minidump) that would like to know more about how > the pstore ram backend carves up the memory regions so it can examine > them itself too. (i.e. it's another "interface" like the pstorefs.) It is not exactly like pstorefs where we can mount and check the logs in next boot. This interface needs physical address and size of the region to be conveyed to the boot firmware via the minidump registration and firmware will dump this region as a blob out of the device on device crash. Blobs can be either plain text like console logs or some thing which needs parser for that reason virtual address is required. > > So we need to provide the mapping back to the overlay. It feels to me > like the logic for this needs to live in the minidump driver itself > (rather than in the pstore RAM backend). Specifically, it wants to know > about all the operational frontends (dmesg, console, ftrace, pmsg) with > their virt & phys addresses and size. > > The frontends are defined via enum pstore_type_id, and the other values > are "normal" types, so it should be possible to move this logic into > minidump instead, leaving a simpler callback. Perhaps something like: > > void pstore_region_defined(enum pstore_type_id, void *virt, > phys_addr_t phys, size_t size); Thanks, i will check on this. > > How the pstore ram backend should know to call this, though, I'm > struggling to find a sensible way. How can it determine if the device > tree region is actually contained by a minidump overlay? I agree, pstore will not have any awareness about minidump adding something inside pstore does not look good if it can provide API which provide this information. One more thing, since minidump is also one of the user of pstore in this case and it may not need the fixed ram addresses to be mentioned in the carve out. Do you think this can be accepted or if not any suggestion https://lore.kernel.org/lkml/1675330081-15029-2-git-send-email-quic_mojha@quicinc.com/ -Mukesh >
On 2/24/2023 1:13 AM, Kees Cook wrote: > On Tue, Feb 21, 2023 at 04:55:13PM +0530, Mukesh Ojha wrote: >> There are system which does not uses pstore directly but >> may have the interest in the context saved by pstore. >> Register pstore regions with minidump so that it get >> dumped on minidump collection. > > Okay, so, this is a really interesting case -- it's a RAM backend that > is already found on a system by pstore via device tree, but there is > _another_ RAM overlay (minidump) that would like to know more about how > the pstore ram backend carves up the memory regions so it can examine > them itself too. (i.e. it's another "interface" like the pstorefs.) > > So we need to provide the mapping back to the overlay. It feels to me > like the logic for this needs to live in the minidump driver itself > (rather than in the pstore RAM backend). Specifically, it wants to know > about all the operational frontends (dmesg, console, ftrace, pmsg) with > their virt & phys addresses and size. > > The frontends are defined via enum pstore_type_id, and the other values > are "normal" types, so it should be possible to move this logic into > minidump instead, leaving a simpler callback. Perhaps something like: > > void pstore_region_defined(enum pstore_type_id, void *virt, > phys_addr_t phys, size_t size); > > How the pstore ram backend should know to call this, though, I'm > struggling to find a sensible way. How can it determine if the device > tree region is actually contained by a minidump overlay? Do you think, if qcom_minidump_ready() can be used which checks minidump readiness ? -Mukesh >
diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index ade66db..038da1a 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -20,6 +20,7 @@ #include <linux/compiler.h> #include <linux/of.h> #include <linux/of_address.h> +#include <soc/qcom/minidump.h> #include "internal.h" #include "ram_internal.h" @@ -714,6 +715,74 @@ static int ramoops_parse_dt(struct platform_device *pdev, return 0; } +#if IS_ENABLED(CONFIG_QCOM_MINIDUMP) +static int ramoops_qcom_minidump_register(struct ramoops_context *cxt) +{ + struct qcom_minidump_region pstore_entry; + struct persistent_ram_zone *prz; + int ret = 0; + int i; + + for (i = 0; i < cxt->max_dump_cnt; i++) { + prz = cxt->dprzs[i]; + scnprintf(pstore_entry.name, sizeof(pstore_entry.name), + "KDMESG%d", i); + pstore_entry.virt_addr = prz->vaddr; + pstore_entry.phys_addr = prz->paddr; + pstore_entry.size = prz->size; + ret = qcom_minidump_region_register(&pstore_entry); + if (ret < 0) { + pr_err("failed to add dmesg in minidump: err: %d\n", ret); + return ret; + } + } + + if (cxt->console_size) { + prz = cxt->cprz; + strlcpy(pstore_entry.name, "KCONSOLE", sizeof(pstore_entry.name)); + pstore_entry.virt_addr = prz->vaddr; + pstore_entry.phys_addr = prz->paddr; + pstore_entry.size = prz->size; + ret = qcom_minidump_region_register(&pstore_entry); + if (ret < 0) { + pr_err("failed to add console in minidump: err: %d\n", ret); + return ret; + } + } + + for (i = 0; i < cxt->max_ftrace_cnt; i++) { + prz = cxt->fprzs[i]; + scnprintf(pstore_entry.name, sizeof(pstore_entry.name), + "KFTRACE%d", i); + pstore_entry.virt_addr = prz->vaddr; + pstore_entry.phys_addr = prz->paddr; + pstore_entry.size = prz->size; + ret = qcom_minidump_region_register(&pstore_entry); + if (ret < 0) { + pr_err("failed to add ftrace in minidump: err: %d\n", ret); + return ret; + } + } + + if (cxt->pmsg_size) { + prz = cxt->mprz; + strlcpy(pstore_entry.name, "KPMSG", sizeof(pstore_entry.name)); + pstore_entry.virt_addr = prz->vaddr; + pstore_entry.phys_addr = prz->paddr; + pstore_entry.size = prz->size; + ret = qcom_minidump_region_register(&pstore_entry); + if (ret < 0) { + pr_err("failed to add pmsg in minidump: err: %d\n", ret); + return ret; + } + } + + return ret; +} +#else +static int ramoops_qcom_minidump_register(struct ramoops_context *cxt) { return 0; } +#endif + static int ramoops_probe(struct platform_device *pdev) { struct device *dev = &pdev->dev; @@ -724,6 +793,10 @@ static int ramoops_probe(struct platform_device *pdev) phys_addr_t paddr; int err = -EINVAL; + err = qcom_minidump_ready(); + if (err && err != -ENODEV) + return -EPROBE_DEFER; + /* * Only a single ramoops area allowed at a time, so fail extra * probes. @@ -841,6 +914,10 @@ static int ramoops_probe(struct platform_device *pdev) } } + err = ramoops_qcom_minidump_register(cxt); + if (err < 0) + goto fail_buf; + err = pstore_register(&cxt->pstore); if (err) { pr_err("registering with pstore failed\n");
There are system which does not uses pstore directly but may have the interest in the context saved by pstore. Register pstore regions with minidump so that it get dumped on minidump collection. Signed-off-by: Mukesh Ojha <quic_mojha@quicinc.com> --- fs/pstore/ram.c | 77 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 77 insertions(+)