| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <8b0d2b06-70ae-4afa-bd04-13eaa4280c03@quicinc.com>
Date: Mon, 11 Sep 2023 10:52:26 +0530
From: Pavan Kondeti <quic_pkondeti@...cinc.com>
To: Mukesh Ojha <quic_mojha@...cinc.com>
CC: <corbet@....net>, <agross@...nel.org>, <andersson@...nel.org>,
<konrad.dybcio@...aro.org>, <robh+dt@...nel.org>,
<krzysztof.kozlowski+dt@...aro.org>, <conor+dt@...nel.org>,
<keescook@...omium.org>, <tony.luck@...el.com>,
<gpiccoli@...lia.com>, <mathieu.poirier@...aro.org>,
<catalin.marinas@....com>, <will@...nel.org>,
<linus.walleij@...aro.org>, <andy.shevchenko@...il.com>,
<vigneshr@...com>, <nm@...com>, <matthias.bgg@...il.com>,
<kgene@...nel.org>, <alim.akhtar@...sung.com>,
<bmasney@...hat.com>, <quic_tsoni@...cinc.com>,
<linux-doc@...r.kernel.org>, <linux-kernel@...r.kernel.org>,
<linux-arm-msm@...r.kernel.org>, <linux-hardening@...r.kernel.org>,
<linux-remoteproc@...r.kernel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<linux-gpio@...r.kernel.org>, <linux-mediatek@...ts.infradead.org>,
<linux-samsung-soc@...r.kernel.org>, <kernel@...cinc.com>
Subject: Re: [PATCH v5 08/17] arm64: mm: Add dynamic ramoops region support
through command line
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@...cinc.com>
> ---
> 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
>
Powered by blists - more mailing lists