On Thu, Oct 08, 2020 at 09:49:46AM -0700, Florian Fainelli wrote:
>
>
> On 10/8/2020 8:54 AM, Serge Semin wrote:
> > On Thu, Oct 08, 2020 at 04:30:35PM +0100, Maciej W. Rozycki wrote:
> > > On Thu, 8 Oct 2020, Serge Semin wrote:
> > >
> > > > 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?
> > >
> >
> > > 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)
>
> AFAICT DTC does not do any validation that regions you declare in
> /memreserve or /reserved-memory are within the 'reg' property defined for
> the /memory node.
> Not that it could not but that goes a little beyond is
> compiler job.
You are probably right, but it does the #{address,size}-cells validations
against the subnode' "reg" properties. It even does I2C controllers subnodes
"reg" validation so they wouldn't be greater than 7 or 10 bits wide. It also
does some magic checks of SPI controllers subnodes. See scripts/dtc/checks.c for
details. So I thought DTC might do some memory/reserved-memory validations too.
Apparently it doesn't. On the other hand it would probably be pointless, since a
system bootloader may change the "memory" node' "reg" value anyway if a platform
has a variable memory layout. So any validation being passed at the DTS compile
time wouldn't surely mean the memory/reserved-memory nodes coherency at the
system boot time.
>
> The kernel ought to be able to do that validation through memblock but there
> could be valid use cases behind declaring a reserved memory region that is
> not backed by a corresponding DRAM region. For instance if you hotplugged
> memory through the sysfs probe interface, and that memory was not initially
> declared in the Device Tree, but there were reserved regions within that
> hot-plugged range that you would have to be aware of, then this would break.
Yeah, it seems to me all hot-pluggable regions are also marked as reserved. So
hot-plugging a memory device behind the manually reserved regions (reserved by
means of the DT reserved-memory/memreserve nodes) will unreserve them, which
most likely isn't what has been originally wanted.
Alas I don't have any hardware with hot-pluggable memory device to check out the
problem availability.
>
> >
> > 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?
>
> Would we be breaking any use case if we stopped allowing reserved region
> that are not part of DRAM being declared?
Hm, good question. I don't really know. But AFAICS from the reserved-memory node
DT bindings the reserved regions can be used to declare both normal RAM and some
vendor-specific regions. The later one theoretically can be some resource, bus or
memory-mapped device thing especially if it's marked with "no-map" boolean
property.
---
Getting back to the topic. In the MIPS-specific early_init_dt_reserve_memory_arch()
method (which is called for every region declared in reserved-memory/memreserve nodes)
we currently consider the reserved region as DRAM if 'no-map' property isn't specified.
The main question here is whether it is correct...
-Sergey
> --
> Florian
