[Skiboot] [PATCH] hdata/memory.c: Fix "Inconsistent MSAREA" warnings

Oliver O'Halloran oohall at gmail.com
Wed Apr 29 09:32:19 AEST 2020


On Mon, Apr 27, 2020 at 7:54 AM Klaus Heinrich Kiwi
<klaus at linux.vnet.ibm.com> wrote:
>
> Oliver,
>
>   I was thinking a bit about this and decided to send a simple 'trivial
> fix' since a more complete one would require some re-work I think
> (besides, a trivial fix should apply cleanly on skiboot-6.6).
>
> Right we're creating memory-buffer@<min-addr> nodes under root, and
> those min-addr are calculated as part of the mmio ranges structure.
>
> If we wanted to add a add_memory_controller_p9p() function, wouldn't we
> want to re-use the same memory-buffer@ nodes to add the address-range
> attributes? The amount of rework for us to be able to do that seems
> significant and not so clear unless we join add_memory_controller_p9p()
> with add_memory_buffer_mmio()

It doesn't look like a significant rework would be required. Return
the dt node from add_buffer_mmio and pass that to
add_memory_controller() and you're 90% done.

> Perhaps we should make things clearer and add
> memory-buffer@<physical-chip-id> when parsing the address range array,
> and add mmio@<min-addr> nodes underneath them when parsing the mmio range?

The address space defined by the top level DT node is the system
physical address space. Any nodes directly under the root need to have
a unit address (the @<addr> bit) which is a system physical address.
Adding nodes with thing@<random_index> is broken and although we do
that in a few places we shouldn't be and I don't want to add any more.
This is all covered in the DT spec: https://www.devicetree.org/

> If you agree, I can try helping with that implementation..

Preemptive nak

Oliver


More information about the Skiboot mailing list