From: Thiago Jung Bauermann <bauerman@linux.ibm.com>
To: Hari Bathini <hbathini@linux.ibm.com>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
Andrew Morton <akpm@linux-foundation.org>,
Pingfan Liu <piliu@redhat.com>,
Kexec-ml <kexec@lists.infradead.org>,
Mimi Zohar <zohar@linux.ibm.com>,
Nayna Jain <nayna@linux.ibm.com>, Petr Tesarik <ptesarik@suse.cz>,
Mahesh J Salgaonkar <mahesh@linux.ibm.com>,
Sourabh Jain <sourabhjain@linux.ibm.com>,
lkml <linux-kernel@vger.kernel.org>,
linuxppc-dev <linuxppc-dev@ozlabs.org>,
Eric Biederman <ebiederm@xmission.com>,
Dave Young <dyoung@redhat.com>, Vivek Goyal <vgoyal@redhat.com>
Subject: Re: [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel
Date: Wed, 15 Jul 2020 19:52:12 -0300 [thread overview]
Message-ID: <87365s9ysj.fsf@morokweng.localdomain> (raw)
In-Reply-To: <159466091925.24747.6840028682768745598.stgit@hbathini.in.ibm.com>
Hari Bathini <hbathini@linux.ibm.com> writes:
> /**
> + * get_usable_memory_ranges - Get usable memory ranges. This list includes
> + * regions like crashkernel, opal/rtas & tce-table,
> + * that kdump kernel could use.
> + * @mem_ranges: Range list to add the memory ranges to.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int get_usable_memory_ranges(struct crash_mem **mem_ranges)
> +{
> + int ret;
> +
> + /* First memory block & crashkernel region */
> + ret = add_mem_range(mem_ranges, 0, crashk_res.end + 1);
This is a bit surprising. I guess I don't have a complete big picture of
the patch series yet. What prevents the crashkernel from using memory at
the [0, _end] range and overwriting the crashed kernel's memory?
Shouldn't the above range start at crashk_res.start?
> + if (ret)
> + goto out;
> +
> + ret = add_rtas_mem_range(mem_ranges);
> + if (ret)
> + goto out;
> +
> + ret = add_opal_mem_range(mem_ranges);
> + if (ret)
> + goto out;
> +
> + ret = add_tce_mem_ranges(mem_ranges);
> +out:
> + if (ret)
> + pr_err("Failed to setup usable memory ranges\n");
> + return ret;
> +}
> +
> +/**
> * __locate_mem_hole_top_down - Looks top down for a large enough memory hole
> * in the memory regions between buf_min & buf_max
> * for the buffer. If found, sets kbuf->mem.
> @@ -261,6 +305,322 @@ static int locate_mem_hole_bottom_up_ppc64(struct kexec_buf *kbuf,
> }
>
> /**
> + * check_realloc_usable_mem - Reallocate buffer if it can't accommodate entries
> + * @um_info: Usable memory buffer and ranges info.
> + * @cnt: No. of entries to accommodate.
> + *
> + * Returns 0 on success, negative errno on error.
It actually returns the buffer on success, and NULL on error.
> + */
> +static uint64_t *check_realloc_usable_mem(struct umem_info *um_info, int cnt)
> +{
> + void *tbuf;
> +
> + if (um_info->size >=
> + ((um_info->idx + cnt) * sizeof(*(um_info->buf))))
> + return um_info->buf;
> +
> + um_info->size += MEM_RANGE_CHUNK_SZ;
> + tbuf = krealloc(um_info->buf, um_info->size, GFP_KERNEL);
> + if (!tbuf) {
> + um_info->size -= MEM_RANGE_CHUNK_SZ;
> + return NULL;
> + }
> +
> + memset(tbuf + um_info->idx, 0, MEM_RANGE_CHUNK_SZ);
> + return tbuf;
> +}
<snip>
> +/**
> + * get_node_path - Get the full path of the given node.
> + * @dn: Node.
> + * @path: Updated with the full path of the node.
> + *
> + * Returns nothing.
> + */
> +static void get_node_path(struct device_node *dn, char *path)
> +{
> + if (!dn)
> + return;
> +
> + get_node_path(dn->parent, path);
Is it ok to do recursion in the kernel? In this case I believe it's not
problematic since the maximum call depth will be the maximum depth of a
device tree node which shouldn't be too much. Also, there are no local
variables in this function. But I thought it was worth mentioning.
> + sprintf(path, "/%s", dn->full_name);
> +}
> +
> +/**
> + * get_node_pathlen - Get the full path length of the given node.
> + * @dn: Node.
> + *
> + * Returns the length of the full path of the node.
> + */
> +static int get_node_pathlen(struct device_node *dn)
> +{
> + int len = 0;
> +
> + while (dn) {
> + len += strlen(dn->full_name) + 1;
> + dn = dn->parent;
> + }
> + len++;
> +
> + return len;
> +}
> +
> +/**
> + * add_usable_mem_property - Add usable memory property for the given
> + * memory node.
> + * @fdt: Flattened device tree for the kdump kernel.
> + * @dn: Memory node.
> + * @um_info: Usable memory buffer and ranges info.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int add_usable_mem_property(void *fdt, struct device_node *dn,
> + struct umem_info *um_info)
> +{
> + int n_mem_addr_cells, n_mem_size_cells, node;
> + int i, len, ranges, cnt, ret;
> + uint64_t base, end, *buf;
> + const __be32 *prop;
> + char *pathname;
> +
> + /* Allocate memory for node path */
> + pathname = kzalloc(ALIGN(get_node_pathlen(dn), 8), GFP_KERNEL);
> + if (!pathname)
> + return -ENOMEM;
> +
> + /* Get the full path of the memory node */
> + get_node_path(dn, pathname);
> + pr_debug("Memory node path: %s\n", pathname);
> +
> + /* Now that we know the path, find its offset in kdump kernel's fdt */
> + node = fdt_path_offset(fdt, pathname);
> + if (node < 0) {
> + pr_err("Malformed device tree: error reading %s\n",
> + pathname);
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + /* Get the address & size cells */
> + n_mem_addr_cells = of_n_addr_cells(dn);
> + n_mem_size_cells = of_n_size_cells(dn);
> + pr_debug("address cells: %d, size cells: %d\n", n_mem_addr_cells,
> + n_mem_size_cells);
> +
> + um_info->idx = 0;
> + buf = check_realloc_usable_mem(um_info, 2);
> + if (!buf) {
> + ret = -ENOMEM;
> + goto out;
> + }
> +
> + um_info->buf = buf;
> +
> + prop = of_get_property(dn, "reg", &len);
> + if (!prop || len <= 0) {
> + ret = 0;
> + goto out;
> + }
> +
> + /*
> + * "reg" property represents sequence of (addr,size) duples
s/duples/tuples/ ?
> + * each representing a memory range.
> + */
> + ranges = (len >> 2) / (n_mem_addr_cells + n_mem_size_cells);
> +
> + for (i = 0; i < ranges; i++) {
> + base = of_read_number(prop, n_mem_addr_cells);
> + prop += n_mem_addr_cells;
> + end = base + of_read_number(prop, n_mem_size_cells) - 1;
You need to `prop += n_mem_size_cells` here.
> +
> + ret = add_usable_mem(um_info, base, end, &cnt);
> + if (ret) {
> + ret = ret;
> + goto out;
> + }
> + }
> +
> + /*
> + * No kdump kernel usable memory found in this memory node.
> + * Write (0,0) duple in linux,usable-memory property for
s/duple/tuple/ ?
> + * this region to be ignored.
> + */
> + if (um_info->idx == 0) {
> + um_info->buf[0] = 0;
> + um_info->buf[1] = 0;
> + um_info->idx = 2;
> + }
> +
> + ret = fdt_setprop(fdt, node, "linux,usable-memory", um_info->buf,
> + (um_info->idx * sizeof(*(um_info->buf))));
> +
> +out:
> + kfree(pathname);
> + return ret;
> +}
--
Thiago Jung Bauermann
IBM Linux Technology Center
next prev parent reply other threads:[~2020-07-15 22:52 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-13 17:20 [PATCH v3 00/12] ppc64: enable kdump support for kexec_file_load syscall Hari Bathini
2020-07-13 17:20 ` [PATCH v3 01/12] kexec_file: allow archs to handle special regions while locating memory hole Hari Bathini
2020-07-14 21:00 ` Thiago Jung Bauermann
2020-07-13 17:21 ` [PATCH v3 02/12] powerpc/kexec_file: mark PPC64 specific code Hari Bathini
2020-07-16 1:49 ` Thiago Jung Bauermann
2020-07-17 4:46 ` Hari Bathini
2020-07-17 18:34 ` Thiago Jung Bauermann
2020-07-13 17:21 ` [PATCH v3 03/12] powerpc/kexec_file: add helper functions for getting memory ranges Hari Bathini
2020-07-14 23:49 ` Thiago Jung Bauermann
2020-07-16 21:08 ` Hari Bathini
2020-07-17 4:32 ` Hari Bathini
2020-07-17 20:00 ` Hari Bathini
2020-07-13 17:21 ` [PATCH v3 04/12] ppc64/kexec_file: avoid stomping memory used by special regions Hari Bathini
2020-07-15 2:39 ` Thiago Jung Bauermann
2020-07-16 5:58 ` Thiago Jung Bauermann
2020-07-16 21:09 ` Hari Bathini
2020-07-16 21:59 ` Thiago Jung Bauermann
2020-07-13 17:21 ` [PATCH v3 05/12] powerpc/drmem: make lmb walk a bit more flexible Hari Bathini
2020-07-15 3:50 ` Thiago Jung Bauermann
2020-07-16 21:09 ` Hari Bathini
2020-07-16 22:01 ` Thiago Jung Bauermann
2020-07-13 17:22 ` [PATCH v3 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel Hari Bathini
2020-07-15 22:52 ` Thiago Jung Bauermann [this message]
2020-07-16 21:10 ` Hari Bathini
2020-07-16 22:03 ` Thiago Jung Bauermann
2020-07-17 4:17 ` Hari Bathini
2020-07-13 17:22 ` [PATCH v3 07/12] ppc64/kexec_file: add support to relocate purgatory Hari Bathini
2020-07-16 0:20 ` Thiago Jung Bauermann
2020-07-16 21:11 ` Hari Bathini
2020-07-16 22:12 ` Thiago Jung Bauermann
2020-07-13 17:22 ` [PATCH v3 08/12] ppc64/kexec_file: setup the stack for purgatory Hari Bathini
2020-07-16 0:35 ` Thiago Jung Bauermann
2020-07-16 1:40 ` Thiago Jung Bauermann
2020-07-13 17:22 ` [PATCH v3 09/12] ppc64/kexec_file: setup backup region for kdump kernel Hari Bathini
2020-07-16 1:38 ` Thiago Jung Bauermann
2020-07-16 21:10 ` Hari Bathini
2020-07-16 22:06 ` Thiago Jung Bauermann
2020-07-13 17:23 ` [PATCH v3 10/12] ppc64/kexec_file: prepare elfcore header for crashing kernel Hari Bathini
2020-07-16 2:22 ` Thiago Jung Bauermann
2020-07-16 21:07 ` Hari Bathini
2020-07-16 21:57 ` Thiago Jung Bauermann
2020-07-13 17:23 ` [PATCH v3 11/12] ppc64/kexec_file: add appropriate regions for memory reserve map Hari Bathini
2020-07-16 2:27 ` Thiago Jung Bauermann
2020-07-13 17:23 ` [PATCH v3 12/12] ppc64/kexec_file: fix kexec load failure with lack of memory hole Hari Bathini
2020-07-16 5:43 ` Thiago Jung Bauermann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87365s9ysj.fsf@morokweng.localdomain \
--to=bauerman@linux.ibm.com \
--cc=akpm@linux-foundation.org \
--cc=dyoung@redhat.com \
--cc=ebiederm@xmission.com \
--cc=hbathini@linux.ibm.com \
--cc=kexec@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linuxppc-dev@ozlabs.org \
--cc=mahesh@linux.ibm.com \
--cc=mpe@ellerman.id.au \
--cc=nayna@linux.ibm.com \
--cc=piliu@redhat.com \
--cc=ptesarik@suse.cz \
--cc=sourabhjain@linux.ibm.com \
--cc=vgoyal@redhat.com \
--cc=zohar@linux.ibm.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).