Re: [PATCH v4 06/12] ppc64/kexec_file: restrict memory usage of kdump kernel

On 24/07/20 5:36 am, Thiago Jung Bauermann wrote:

Hari Bathini [off-list ref] writes:

quoted

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 <hbathini@linux.ibm.com>
Tested-by: Pingfan Liu <redacted>
---

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.

Good idea.

quoted

* Fixed prop pointer update in add_usable_mem_property() and changed
  duple to tuple as suggested by Thiago.

<snip>

quoted

+/**
+ * get_node_pathlen - Get the full path length of the given node.
+ * @dn:               Node.
+ *
+ * Also, counts '/' at the end of the path.
+ * For example, /memory@0 will be "/memory@0/\0" => 11 bytes.

Wouldn't this function return 10 in the case of /memory@0?

Actually, it does return 11. +1 while returning is for counting %NUL.
On top of that we count an extra '/' for root node.. so, it ends up as 11.
('/'memory@0'/''\0'). Note the extra '/' before '\0'. Let me handle root node
separately. That should avoid the confusion.

quoted

+ *
+ * Returns the string length of the node's full path.
+ */

Maybe it's me (by analogy with strlen()), but I would expect "string
length" to not include the terminating \0. I suggest renaming the
function to something like get_node_path_size() and do s/length/size/ in
the comment above if it's supposed to count the terminating \0.

Sure, will update the function name.

Thanks
Hari