Re: [PATCH v2] MIPS: replace add_memory_region with memblock

On Thu, Oct 08, 2020 at 05:56:17PM +0100, Maciej W. Rozycki wrote:

On Thu, 8 Oct 2020, Serge Semin wrote:

quoted

quoted

quoted

At least I don't see a decent reason to preserve them. The memory registration
method does nearly the same sanity checks. The memory reservation function
defers a bit in adding the being reserved memory first. That seems redundant,
since the reserved memory won't be available for the system anyway. Do I miss
something?

quoted

 At the very least it serves informational purposes as it shows up in 
/proc/iomem.

I thought about that, but /proc/iomem prints the System RAM up. Adding the reserved
memory regions to be just memory region first still seem redundant, since
reserving a non-reflected in memory region most likely indicates an erroneous
dts. I failed to find that, but do the kernel or DTC make sure that the reserved
memory regions has actual memory behind? (At least in the framework of the
memblock.memory vs memblock.reserved arrays or in the DT source file)

 Reserved regions need not be RAM or a memory-like device.  

Agree. My statement about considering as error the situation when the
reserved-memory is declared with no backed DRAM region was most likely
wrong. But in this case what we currently do and what Thomas suggest to keep
doing in the framework of the MIPS-specific early_init_dt_reserve_memory_arch()
is probably incorrect, since the method makes sure that any added reserved
memory region is actually backed with DRAM (called both memblock_add() and
memblock_reserve() for such regions). None of the other platforms is noticed to
execute the same pattern.

They could be 
floating bus even.  Here's an example from my x86 laptop where all kinds 
of stuff is marked reserved:

00000000-00000fff : Reserved
00001000-0009cfff : System RAM
0009d000-0009ffff : Reserved

AFAICS from the ./arch/x86/kernel/e820.c code, the regions marked as "Reserved"
aren't mem-mapped by the kernel, since they are skipped in the method
e820__memblock_setup(), which is responsible for the x86-specific memory mapping
data conversion to the memblock. So the system doesn't consider them as a RAM
resource. Note each normal memory is supposed to be added to the memblock, so to
be then used by the buddy allocator for normal memory allocations.

Basically the x86-specific "Reserved" regions are similar to the regions added
by means of the "reserved-memory" DT sub-node with "no-map" property specified.
The problem in the topic is whether we must or must not make sure that each
reserved-memory region with no "no-map" property considered as normal memory
region backed with DRAM. The rest of the platforms currently just register the
"reserved-memory" regions in memblock.reserved with no checking whether they are
backed with corresponding DRAM region and with no adding them to the
memblock.memory pool. So most likely we ought to do the same in MIPS.

...

Actually another reason to mark regions reserved is to prevent them from 
being claimed by the wrong driver or, perhaps more importantly, used for 
assigning bus address ranges to hardware resources (BAR programming).

Right, AFAICS that's what the "reserved-memory" DT node has been introduced for.
By using it we can create a DMA/CMA memory pool fully dedicated for a particular
device, or just completely remove a memory region from the kernel virtual
mapping framework if instead of providing an access to the System RAM a region
is remapped to IO to/from some hardware resource.

-Sergey

quoted

I also don't see the other platforms doing that, since the MIPS arch only
redefines these methods. So if a problem of adding a reserved memory with
possible no real memory behind exist, it should be fixed in the cross-platform
basis, don't you think?

 I think doing things in a generic way where possible is surely desired, 
however platforms have different ways to discover resources and I can't 
see offhand how this could be unified.  I haven't look at that code for a 
while now, so I can't be more specific offhand.

  Maciej

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel