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

[Hari Bathini]

On 24/07/20 5:36 am, Thiago Jung Bauermann wrote:
> 
> Hari Bathini <hbathini@xxxxxxxxxxxxx> 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 <hbathini@xxxxxxxxxxxxx>
>> Tested-by: Pingfan Liu <piliu@xxxxxxxxxx>
>> ---
>>
>> 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.
> 
>> * Fixed prop pointer update in add_usable_mem_property() and changed
>>   duple to tuple as suggested by Thiago.
> 
> <snip>
> 
>> +/**
>> + * 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.

>> + *
>> + * 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