Hi James, Rob, On 2020/5/30 0:11, James Morse wrote: > Hi guys, > > On 26/05/2020 22:18, Rob Herring wrote: >> On Fri, May 22, 2020 at 11:24:11AM +0800, chenzhou wrote: >>> On 2020/5/21 21:29, Rob Herring wrote: >>>> On Thu, May 21, 2020 at 3:35 AM Chen Zhou <chenzhou10@xxxxxxxxxx> wrote: >>>>> Add documentation for DT property used by arm64 kdump: >>>>> linux,low-memory-range. >>>>> "linux,low-memory-range" is an another memory region used for crash >>>>> dump kernel devices. >>>>> diff --git a/Documentation/devicetree/bindings/chosen.txt b/Documentation/devicetree/bindings/chosen.txt >>>>> index 45e79172a646..bfe6fb6976e6 100644 >>>>> --- a/Documentation/devicetree/bindings/chosen.txt >>>>> +++ b/Documentation/devicetree/bindings/chosen.txt >>>>> +linux,low-memory-range >>>>> +---------------------- >>>>> +This property (arm64 only) holds a base address and size, describing a >>>>> +limited region below 4G. Similar to "linux,usable-memory-range", it is >>>>> +an another memory range which may be considered available for use by the >>>>> +kernel. >>>> Why can't you just add a range to "linux,usable-memory-range"? It >>>> shouldn't be hard to figure out which part is below 4G. >>> The comments from James: >>> Won't this break if your kdump kernel doesn't know what the extra parameters are? >>> Or if it expects two ranges, but only gets one? These DT properties should be treated as >>> ABI between kernel versions, we can't really change it like this. >>> >>> I think the 'low' region is an optional-extra, that is never mapped by the first kernel. I >>> think the simplest thing to do is to add an 'linux,low-memory-range' that we >>> memblock_add() after memblock_cap_memory_range() has been called. >>> If its missing, or the new kernel doesn't know what its for, everything keeps working. >> >> I don't think there's a compatibility issue here though. The current >> kernel doesn't care if the property is longer than 1 base+size. It only >> checks if the size is less than 1 base+size. > Aha! I missed that. > > >> And yes, we can rely on >> that implementation detail. It's only an ABI if an existing user >> notices. >> >> Now, if the low memory is listed first, then an older kdump kernel >> would get a different memory range. If that's a problem, then define >> that low memory goes last. > This first entry would need to be the 'crashkernel' range where the kdump kernel is > placed, otherwise an older kernel won't boot. The rest can be optional extras, as long as > we are tolerant of it being missing... How about like this: 1. The low memory region remained as "Crash kernel (low)". 2. Userspace will find "Crash kernel" and "Crash kernel (low)" region in /proc/iomem, and add "Crash kernel (low)" as the last range of property "linux,usable-memory-range". Thanks, Chen Zhou > > I'll try and look at the rest of this series on Monday, > > > Thanks, > > James > > . >