Re: [RESEND PATCH v5 06/11] ppc64/kexec_file: restrict memory usage of kdump kernel
On 28/07/20 7:14 pm, Michael Ellerman wrote:
Hari Bathini writes:
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index 2df6f4273ddd..8df085a22fd7 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -17,9 +17,21 @@
#include
#include
#include
+#include
#include
+#include
+#include
#include
+struct umem_info {
+ uint64_t *buf; /* data buffer for usable-memory property */
+ uint32_t idx; /* current index */
+ uint32_t size; /* size allocated for the data buffer */
Use kernel types please, u64, u32.
+ /* usable memory ranges to look up */
+ const struct crash_mem *umrngs;
"umrngs".
Given it's part of the umem_info struct could it just be "ranges"?
True. Actually, having crash_mem_range *ranges + u32 nr_ranges and
populating them seems better. Will do that..
+ return NULL;
+ }
um_info->size = new_size;
+
+ memset(tbuf + um_info->idx, 0, MEM_RANGE_CHUNK_SZ);
Just pass __GFP_ZERO to krealloc?
There are patches submitted to stable fixing a few modules that use
krealloc with __GFP_ZERO. Also, this zeroing is not really needed.
I will drop the memset instead..
Thanks
Hari
Re: [RESEND PATCH v5 06/11] ppc64/kexec_file: restrict memory usage of kdump kernel
Hari Bathini writes:
> diff --git a/arch/powerpc/kexec/file_load_64.c
> b/arch/powerpc/kexec/file_load_64.c
> index 2df6f4273ddd..8df085a22fd7 100644
> --- a/arch/powerpc/kexec/file_load_64.c
> +++ b/arch/powerpc/kexec/file_load_64.c
> @@ -17,9 +17,21 @@
> #include
> #include
> #include
> +#include
> #include
> +#include
> +#include
> #include
>
> +struct umem_info {
> + uint64_t *buf; /* data buffer for usable-memory property */
> + uint32_t idx; /* current index */
> + uint32_t size; /* size allocated for the data buffer */
Use kernel types please, u64, u32.
> + /* usable memory ranges to look up */
> + const struct crash_mem *umrngs;
"umrngs".
Given it's part of the umem_info struct could it just be "ranges"?
> +};
> +
> const struct kexec_file_ops * const kexec_file_loaders[] = {
> &kexec_elf64_ops,
> NULL
> @@ -74,6 +86,42 @@ static int get_exclude_memory_ranges(struct crash_mem
> **mem_ranges)
> return ret;
> }
>
> +/**
> + * 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;
> +
> + /*
> + * prom code doesn't take kindly to missing low memory. So, add
I don't know what that's referring to, "prom code" is too vague.
> + * [0, crashk_res.end] instead of [crashk_res.start, crashk_res.end]
> + * to keep it happy.
> + */
> + ret = add_mem_range(mem_ranges, 0, crashk_res.end + 1);
> + 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
> @@ -273,6 +321,382 @@ static int locate_mem_hole_bottom_up_ppc64(struct
> kexec_buf *kbuf,
> return ret;
> }
>
> +/**
> + * 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.
> + *
> + * Frees up the old buffer if memory reallocation fails.
> + *
> + * Returns buffer on success, 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;
This is awkward.
AFAICS you only use um_info->size here, so instead why not store the
number of u64s you have space for, as num for example.
Then the above comparison becomes:
if (um_info->num >= (um_info->idx + count))
Then you only have to calculate the size internally here for the
realloc.
> +
> + um_info->size += MEM_RANGE_CHUNK_SZ;
new_size = um_info->size + MEM_RANGE_CHUNK_SZ;
tbuf = krealloc(um_info->buf, new_size, GFP_KERNEL);
> + tbuf = krealloc(um_info->buf, um_info->size, GFP_KERNEL);
> + if (!tbuf) {
> + um_info->size -= MEM_RANGE_CHUNK_SZ;
Then you can drop this.
> + return NULL;
> + }
um_info->size = new_size;
> +
> + memset(tbuf + um_info->idx, 0, MEM_RANGE_CHUNK_SZ);
Just pass __GFP_ZERO to krealloc?
> + return tbuf;
> +}
> +
> +/**
> + * add_usable_mem - Add the usable memory ranges within the given memory
> range
> + * to the buffer
> + * @um_info:Usable memory buffer and ranges info.
> + * @base: Base address of memory range to look for.
> + * @end:End address of memory range to look for.
> + * @cnt:No. of usable memory ranges added to buffer.
One caller never uses this AFAICS.
Couldn't the other caller just compare the um_info->idx before and after
the call, and avoid another pass by reference parameter.
> + *
> + * Returns 0 on success, negative errno on error.
> + */
> +static int add_usable_mem(struct umem_info *um_info, uint64_t base,
> + uint64_t end, int *cnt)
> +{
> + uint64_t loc_base, loc_end, *buf;
> + const struct crash_mem *umrngs;
> + int i, add;
add should be bool.
> + *cnt = 0;
> + umrngs = um_info->umrngs;
> + for (i = 0; i < umrngs->nr_ranges; i++) {
> + add = 0;
> + loc_base = umrngs->ranges[i].start;
> +
Re: [RESEND PATCH v5 06/11] ppc64/kexec_file: restrict memory usage of kdump kernel
Hari Bathini writes: > Kdump kernel, used for capturing the kernel core image, is supposed > to use only specific memory regions to avoid corrupting the image to > be captured. The regions are crashkernel range - the memory reserved > explicitly for kdump kernel, memory used for the tce-table, the OPAL > region and RTAS region as applicable. Restrict kdump kernel memory > to use only these regions by setting up usable-memory DT property. > Also, tell the kdump kernel to run at the loaded address by setting > the magic word at 0x5c. > > Signed-off-by: Hari Bathini > Tested-by: Pingfan Liu I liked the new versions of get_node_path_size() and get_node_path(). Reviewed-by: Thiago Jung Bauermann -- Thiago Jung Bauermann IBM Linux Technology Center
[RESEND PATCH v5 06/11] ppc64/kexec_file: restrict memory usage of kdump kernel
Kdump kernel, used for capturing the kernel core image, is supposed
to use only specific memory regions to avoid corrupting the image to
be captured. The regions are crashkernel range - the memory reserved
explicitly for kdump kernel, memory used for the tce-table, the OPAL
region and RTAS region as applicable. Restrict kdump kernel memory
to use only these regions by setting up usable-memory DT property.
Also, tell the kdump kernel to run at the loaded address by setting
the magic word at 0x5c.
Signed-off-by: Hari Bathini
Tested-by: Pingfan Liu
---
v4 -> v5:
* Renamed get_node_pathlen() function to get_node_path_size() and
handled root node separately to avoid off-by-one error in
calculating string size.
* Updated get_node_path() in line with change in get_node_path_size().
v3 -> v4:
* Updated get_node_path() to be an iterative function instead of a
recursive one.
* Added comment explaining why low memory is added to kdump kernel's
usable memory ranges though it doesn't fall in crashkernel region.
* For correctness, added fdt_add_mem_rsv() for the low memory being
added to kdump kernel's usable memory ranges.
* Fixed prop pointer update in add_usable_mem_property() and changed
duple to tuple as suggested by Thiago.
v2 -> v3:
* Unchanged. Added Tested-by tag from Pingfan.
v1 -> v2:
* Fixed off-by-one error while setting up usable-memory properties.
* Updated add_rtas_mem_range() & add_opal_mem_range() callsites based on
the new prototype for these functions.
arch/powerpc/kexec/file_load_64.c | 478 +
1 file changed, 477 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kexec/file_load_64.c
b/arch/powerpc/kexec/file_load_64.c
index 2df6f4273ddd..8df085a22fd7 100644
--- a/arch/powerpc/kexec/file_load_64.c
+++ b/arch/powerpc/kexec/file_load_64.c
@@ -17,9 +17,21 @@
#include
#include
#include
+#include
#include
+#include
+#include
#include
+struct umem_info {
+ uint64_t *buf; /* data buffer for usable-memory property */
+ uint32_t idx; /* current index */
+ uint32_t size; /* size allocated for the data buffer */
+
+ /* usable memory ranges to look up */
+ const struct crash_mem *umrngs;
+};
+
const struct kexec_file_ops * const kexec_file_loaders[] = {
&kexec_elf64_ops,
NULL
@@ -74,6 +86,42 @@ static int get_exclude_memory_ranges(struct crash_mem
**mem_ranges)
return ret;
}
+/**
+ * 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;
+
+ /*
+* prom code doesn't take kindly to missing low memory. So, add
+* [0, crashk_res.end] instead of [crashk_res.start, crashk_res.end]
+* to keep it happy.
+*/
+ ret = add_mem_range(mem_ranges, 0, crashk_res.end + 1);
+ 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
@@ -273,6 +321,382 @@ static int locate_mem_hole_bottom_up_ppc64(struct
kexec_buf *kbuf,
return ret;
}
+/**
+ * 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.
+ *
+ * Frees up the old buffer if memory reallocation fails.
+ *
+ * Returns buffer on success, 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;
+}
+
+/**
+ * add_usable_mem - Add the usable memory ranges within the given memory range
+ * to the buffer
+ * @um_info:Usable memory buffer and ranges info.
+ * @base: Base address of memory range to look for.
+ * @end:End address of memory range t
