On Sun, Sep 10, 2023 at 01:46:09AM +0530, Mukesh Ojha wrote:
> The reserved memory region for ramoops is assumed to be at a fixed
> and known location when read from the devicetree. This may not be
> required for something like Qualcomm's minidump which is interested
> in knowing addresses of ramoops region but it does not put hard
> requirement of address being fixed as most of it's SoC does not
> support warm reset and does not use pstorefs at all instead it has
> firmware way of collecting ramoops region if it gets to know the
> address and register it with apss minidump table which is sitting
> in shared memory region in DDR and firmware will have access to
> these table during reset and collects it on crash of SoC.
>
> So, add the support of reserving ramoops region to be dynamically
> allocated early during boot if it is request through command line
> via 'dyn_ramoops_size=' and fill up reserved resource structure and
> export the structure, so that it can be read by ramoops driver.
>
This needs to be documented at
Documentation/admin-guide/kernel-parameters.txt
> Signed-off-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>
> ---
> arch/arm64/mm/init.c | 94 ++++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pstore_ram.h | 2 +
> 2 files changed, 96 insertions(+)
>
> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
> index d31c3a9290c5..14d7086758bf 100644
> --- a/arch/arm64/mm/init.c
> +++ b/arch/arm64/mm/init.c
> @@ -31,6 +31,7 @@
> #include <linux/hugetlb.h>
> #include <linux/acpi_iort.h>
> #include <linux/kmemleak.h>
> +#include <linux/pstore_ram.h>
>
> #include <asm/boot.h>
> #include <asm/fixmap.h>
> @@ -73,6 +74,93 @@ phys_addr_t __ro_after_init arm64_dma_phys_limit;
>
> #define DEFAULT_CRASH_KERNEL_LOW_SIZE (128UL << 20)
>
> +#define RAMOOPS_ADDR_HIGH_MAX (PHYS_MASK + 1)
> +
> +/* Location of the reserved area for the dynamic ramoops */
> +struct resource dyn_ramoops_res = {
> + .name = "ramoops",
> + .start = 0,
> + .end = 0,
> + .flags = IORESOURCE_BUSY | IORESOURCE_SYSTEM_RAM,
> + .desc = IORES_DESC_NONE,
> +};
> +EXPORT_SYMBOL(dyn_ramoops_res);
Use EXPORT_SYMBOL_GPL.
> +
> +static int __init parse_dynamic_ramoops(char *cmdline, unsigned long long *size)
> +{
> + const char *name = "dyn_ramoops_size=";
> + char *p = NULL;
> + char *q = NULL;
> + char *tmp;
> +
> + if (!cmdline)
> + return -ENOENT;
> +
> + /* Check for "dyn_ramoops_size" and use the later if there are more */
> + p = strstr(cmdline, name);
> + while (p) {
> + q = p;
> + p = strchr(p, ' ');
> + if (!p)
> + break;
> +
> + p = strstr(p + 1, name);
> + }
> +
> + if (!q) {
> + pr_err("ramoops: No entry found for %s\n", name);
> + return -ENOENT;
> + }
> +
> + p = q + strlen(name);
> + *size = memparse(p, &tmp);
> + if (p == tmp) {
> + pr_err("ramoops: memory value expected\n");
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +static int __init parse_dyn_ramoops_size_dummy(char *arg)
> +{
> + return 0;
> +}
> +early_param("dyn_ramoops_size", parse_dyn_ramoops_size_dummy);
> +
Any reason why we can't parse and cache the size in early param handler
it self?
> +/*
> + * reserve_dynamic_ramoops() - reserves memory for dynamic ramoops
> + *
> + * This enable dynamic reserve memory support for ramoops through
> + * command line.
> + */
> +static void __init reserve_dynamic_ramoops(void)
> +{
> + char *cmdline = boot_command_line;
> + unsigned long long ramoops_base;
> + unsigned long long ramoops_size;
> +
> + if (!IS_ENABLED(CONFIG_PSTORE_RAM))
> + return;
> +
Should not most part of this patch be under CONFIG_PSTORE_RAM?
> + if (parse_dynamic_ramoops(cmdline, &ramoops_size))
> + return;
> +
> + ramoops_base = memblock_phys_alloc_range(ramoops_size, SMP_CACHE_BYTES,
> + 0, RAMOOPS_ADDR_HIGH_MAX);
It may be appropriate to use one of MEMBLOCK_ALLOC_xxx flags for the end
marker.
> + if (!ramoops_base) {
> + pr_err("cannot allocate ramoops dynamic memory (size:0x%llx).\n",
> + ramoops_size);
> + return;
> + }
> +
> + kmemleak_ignore_phys(ramoops_base);
Looks like you need MEMBLOCK_ALLOC_NOLEAKTRACE
> +
> + dyn_ramoops_res.start = ramoops_base;
> + dyn_ramoops_res.end = ramoops_base + ramoops_size - 1;
> + insert_resource(&iomem_resource, &dyn_ramoops_res);
> +}
> +
> static int __init reserve_crashkernel_low(unsigned long long low_size)
> {
> unsigned long long low_base;
> @@ -456,6 +544,12 @@ void __init bootmem_init(void)
> */
> reserve_crashkernel();
>
> + /*
> + * Reserving ramoops region resource dynamically in case it is
> + * requested from command line.
> + */
> + reserve_dynamic_ramoops();
> +
> memblock_dump_all();
> }
>
> diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h
> index 9d65ff94e216..07d700b7649d 100644
> --- a/include/linux/pstore_ram.h
> +++ b/include/linux/pstore_ram.h
> @@ -10,6 +10,8 @@
>
> #include <linux/pstore.h>
>
> +extern struct resource dyn_ramoops_res;
> +
What about other architectures?
> struct persistent_ram_ecc_info {
> int block_size;
> int ecc_size;
> --
> 2.7.4
>
